[Xen-devel] [linux-linus test] 132006: regressions - FAIL
flight 132006 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/132006/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-pvhv2-intel 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-multivcpu 7 xen-bootfail REGR. vs. 125898 test-amd64-amd64-rumprun-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-pair10 xen-boot/src_hostfail REGR. vs. 125898 test-amd64-amd64-amd64-pvgrub 7 xen-bootfail REGR. vs. 125898 test-amd64-amd64-pair11 xen-boot/dst_hostfail REGR. vs. 125898 test-amd64-amd64-xl-qcow2 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-libvirt-vhd 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-pygrub 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-win7-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-libvirt-pair 10 xen-boot/src_host fail REGR. vs. 125898 test-amd64-amd64-libvirt-pair 11 xen-boot/dst_host fail REGR. vs. 125898 test-amd64-i386-examine 8 reboot fail REGR. vs. 125898 test-amd64-i386-xl-xsm7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl7 xen-boot fail REGR. vs. 125898 test-amd64-i386-rumprun-i386 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-shadow 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 125898 test-amd64-i386-freebsd10-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 125898 test-amd64-i386-xl-qemut-win10-i386 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 125898 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 125898 test-amd64-i386-libvirt-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-libvirt-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-libvirt 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-examine 8 reboot fail REGR. vs. 125898 test-amd64-i386-qemut-rhel6hvm-amd 12 guest-start/redhat.repeat fail REGR. vs. 125898 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 7 xen-boot fail REGR. vs. 125898 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-credit1 7 xen-bootfail baseline untested test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 125898 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 125898 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 125898 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 125898 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 125898 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 125898 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 125898 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass
[Xen-devel] [libvirt test] 132027: tolerable all pass - PUSHED
flight 132027 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/132027/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 131978 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 131978 test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass version targeted for testing: libvirt 4ab8447af778e7e9a10fd80feafd30cb26da7c09 baseline version: libvirt 1b3ea6daaf0e8641571b357242e37fd8a1c33a71 Last test of basis 131978 2019-01-16 12:52:17 Z2 days Testing same since 132027 2019-01-17 20:41:55 Z1 days1 attempts People who touched revisions under test: Ján Tomko Peter Krempa jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-armhf-armhf-libvirt pass test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair pass test-armhf-armhf-libvirt-raw pass test-amd64-amd64-libvirt-vhd 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/libvirt.git 1b3ea6daaf..4ab8447af7 4ab8447af778e7e9a10fd80feafd30cb26da7c09 -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 19/21] treewide: add checks for the return value of memblock_alloc*()
Hi Mike, On Wed, Jan 16, 2019 at 03:44:19PM +0200, Mike Rapoport wrote: > Add check for the return value of memblock_alloc*() functions and call > panic() in case of error. > The panic message repeats the one used by panicing memblock allocators with > adjustment of parameters to include only relevant ones. > > The replacement was mostly automated with semantic patches like the one > below with manual massaging of format strings. > > @@ > expression ptr, size, align; > @@ > ptr = memblock_alloc(size, align); > + if (!ptr) > + panic("%s: Failed to allocate %lu bytes align=0x%lx\n", __func__, > size, align); > > Signed-off-by: Mike Rapoport > --- >% > diff --git a/arch/mips/cavium-octeon/dma-octeon.c > b/arch/mips/cavium-octeon/dma-octeon.c > index e8eb60e..db1deb2 100644 > --- a/arch/mips/cavium-octeon/dma-octeon.c > +++ b/arch/mips/cavium-octeon/dma-octeon.c > @@ -245,6 +245,9 @@ void __init plat_swiotlb_setup(void) > swiotlbsize = swiotlb_nslabs << IO_TLB_SHIFT; > > octeon_swiotlb = memblock_alloc_low(swiotlbsize, PAGE_SIZE); > + if (!octeon_swiotlb) > + panic("%s: Failed to allocate %lu bytes align=%lx\n", > + __func__, swiotlbsize, PAGE_SIZE); > > if (swiotlb_init_with_tbl(octeon_swiotlb, swiotlb_nslabs, 1) == -ENOMEM) > panic("Cannot allocate SWIOTLB buffer"); That one should be %zu rather than %lu. The rest looks good, so with that one tweak: Acked-by: Paul Burton # MIPS parts Thanks, Paul ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 12/21] arch: use memblock_alloc() instead of memblock_alloc_from(size, align, 0)
Hi Mike, On Wed, Jan 16, 2019 at 03:44:12PM +0200, Mike Rapoport wrote: > The last parameter of memblock_alloc_from() is the lower limit for the > memory allocation. When it is 0, the call is equivalent to > memblock_alloc(). > > Signed-off-by: Mike Rapoport Acked-by: Paul Burton # MIPS part Thanks, Paul ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [qemu-mainline test] 132032: regressions - FAIL
flight 132032 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/132032/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-xsm6 xen-buildfail REGR. vs. 131842 build-i3866 xen-buildfail REGR. vs. 131842 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail REGR. vs. 131842 build-armhf 6 xen-buildfail REGR. vs. 131842 Tests which did not succeed, but are not blocking: test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1 build-check(1) blocked n/a test-amd64-i386-xl-shadow 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-pvshim 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit1 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-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 131842 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 131842 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass version targeted for testing: qemuu681d61362d3f766a00806b89d6581869041f73cb baseline version: qemuu147923b1a901a0370f83a0f4c58ec1baffef22f0 Last test of basis 131842 2019-01-09 00:37:22 Z 10 days Failing since131892 2019-01-09 23:37:00 Z9 days8 attempts Testing same since 132032 2019-01-17 21:57:41 Z1 days1 attempts People who touched revisions under test: Alex Bennée Alex Williamson Alexandro Sanchez Bach Alexey Kardashevskiy Alistair Francis Andrew Jeffery Anthony PERARD BALATON Zoltan Borislav Petkov Cédric Le Goater Daniel P. Berrangé David Gibson David Hildenbrand Eduardo Habkost Eric Blake Fam Zheng Frediano Ziglio Gerd Hoffmann Greg Kurz Guenter Roeck Joel Stanley John Snow Kashyap Chamarthy Laurent Vivier Laurent Vivier Li Feng Li Qiang Marc-André Lureau Mark Cave-Ayland Michael Clark Michael Roth Palmer Dabbelt Paolo Bonzini Paul Durrant Peng Hao Peter Maydell Philippe
[Xen-devel] [ovmf test] 132024: regressions - FAIL
flight 132024 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/132024/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 129475 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 129475 version targeted for testing: ovmf c68b5649002c2c75633cdf8a949803cacbe8d044 baseline version: ovmf 5ae3184d8c59f7bbb84bad482df6b8020ba58188 Last test of basis 129475 2018-11-05 21:13:11 Z 74 days Failing since129526 2018-11-06 20:49:26 Z 73 days 268 attempts Testing same since 132024 2019-01-17 19:47:24 Z1 days1 attempts People who touched revisions under test: Achin Gupta Alex James Ard Biesheuvel Ashish Singhal Bob Feng bob.c.f...@intel.com BobCF Carsey, Jaben Chasel Chiu Chasel, Chiu Chen A Chen Chu, Maggie Dandan Bi David Wei Derek Lin Eric Dong Eugene Cohen Feng, Bob C Fu Siyuan Gary Lin Hao Wu Jaben Carsey Jagadeesh Ujja Jeff Brasen Jian J Wang Jiaxin Wu Jiewen Yao Laszlo Ersek Leif Lindholm Liming Gao Liu Yu Maggie Chu Marc Zyngier Marcin Wojtas Mike Maslenkin Ming Huang Pedroa Liu Ray Ni Ruiyu Ni Sami Mujawar shenglei Shenglei Zhang Siyuan Fu Songpeng Li Star Zeng Sughosh Ganu Sumit Garg Sun, Zailiang Thomas Abraham Thomas Rydman Ting Ye Tomasz Michalec Vijayenthiran Subramaniam Vladimir Olovyannikov Wang BinX A Wu Jiaxin Ye Ting Yonghong Zhu yuchenlin Zailiang Sun Zhang, Chao B Zhao, ZhiqiangX Zhiju.Fan zhijufan ZhiqiangX Zhao zwei4 jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 fail test-amd64-i386-xl-qemuu-ovmf-amd64 fail 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 6664 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] arm64/xen: fix xen-swiotlb cache flushing
On Thu, 17 Jan 2019, Christoph Hellwig wrote: > Xen-swiotlb hooks into the arm/arm64 arch code through a copy of the > DMA mapping operations stored in the struct device arch data. > > Switching arm64 to use the direct calls for the merged DMA direct / > swiotlb code broke this scheme. Replace the indirect calls with > direct-calls in xen-swiotlb as well to fix this problem. > > Fixes: 356da6d0cd ("dma-mapping: bypass indirect calls for dma-direct") > Reported-by: Julien Grall > Signed-off-by: Christoph Hellwig > --- > arch/arm/include/asm/xen/page-coherent.h | 94 + > arch/arm64/include/asm/device.h| 3 - > arch/arm64/include/asm/xen/page-coherent.h | 76 + > arch/arm64/mm/dma-mapping.c| 4 +- > drivers/xen/swiotlb-xen.c | 4 +- > include/xen/arm/page-coherent.h| 97 +- > 6 files changed, 176 insertions(+), 102 deletions(-) > > diff --git a/arch/arm/include/asm/xen/page-coherent.h > b/arch/arm/include/asm/xen/page-coherent.h > index b3ef061d8b74..2c403e7c782d 100644 > --- a/arch/arm/include/asm/xen/page-coherent.h > +++ b/arch/arm/include/asm/xen/page-coherent.h > @@ -1 +1,95 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _ASM_ARM_XEN_PAGE_COHERENT_H > +#define _ASM_ARM_XEN_PAGE_COHERENT_H > + > +#include > +#include > #include > + > +static inline const struct dma_map_ops *xen_get_dma_ops(struct device *dev) > +{ > + if (dev && dev->archdata.dev_dma_ops) > + return dev->archdata.dev_dma_ops; > + return get_arch_dma_ops(NULL); > +} > + > +static inline void *xen_alloc_coherent_pages(struct device *hwdev, size_t > size, > + dma_addr_t *dma_handle, gfp_t flags, unsigned long attrs) > +{ > + return xen_get_dma_ops(hwdev)->alloc(hwdev, size, dma_handle, flags, > attrs); > +} > + > +static inline void xen_free_coherent_pages(struct device *hwdev, size_t size, > + void *cpu_addr, dma_addr_t dma_handle, unsigned long attrs) > +{ > + xen_get_dma_ops(hwdev)->free(hwdev, size, cpu_addr, dma_handle, attrs); > +} > + > +static inline void xen_dma_map_page(struct device *hwdev, struct page *page, > + dma_addr_t dev_addr, unsigned long offset, size_t size, > + enum dma_data_direction dir, unsigned long attrs) > +{ > + unsigned long page_pfn = page_to_xen_pfn(page); > + unsigned long dev_pfn = XEN_PFN_DOWN(dev_addr); > + unsigned long compound_pages = > + (1< + bool local = (page_pfn <= dev_pfn) && > + (dev_pfn - page_pfn < compound_pages); > + > + /* > + * Dom0 is mapped 1:1, while the Linux page can span across > + * multiple Xen pages, it's not possible for it to contain a > + * mix of local and foreign Xen pages. So if the first xen_pfn > + * == mfn the page is local otherwise it's a foreign page > + * grant-mapped in dom0. If the page is local we can safely > + * call the native dma_ops function, otherwise we call the xen > + * specific function. > + */ > + if (local) > + xen_get_dma_ops(hwdev)->map_page(hwdev, page, offset, size, > dir, attrs); > + else > + __xen_dma_map_page(hwdev, page, dev_addr, offset, size, dir, > attrs); > +} > + > +static inline void xen_dma_unmap_page(struct device *hwdev, dma_addr_t > handle, > + size_t size, enum dma_data_direction dir, unsigned long attrs) > +{ > + unsigned long pfn = PFN_DOWN(handle); > + /* > + * Dom0 is mapped 1:1, while the Linux page can be spanned accross > + * multiple Xen page, it's not possible to have a mix of local and > + * foreign Xen page. Dom0 is mapped 1:1, so calling pfn_valid on a > + * foreign mfn will always return false. If the page is local we can > + * safely call the native dma_ops function, otherwise we call the xen > + * specific function. > + */ > + if (pfn_valid(pfn)) { > + if (xen_get_dma_ops(hwdev)->unmap_page) > + xen_get_dma_ops(hwdev)->unmap_page(hwdev, handle, size, > dir, attrs); > + } else > + __xen_dma_unmap_page(hwdev, handle, size, dir, attrs); > +} > + > +static inline void xen_dma_sync_single_for_cpu(struct device *hwdev, > + dma_addr_t handle, size_t size, enum dma_data_direction dir) > +{ > + unsigned long pfn = PFN_DOWN(handle); > + if (pfn_valid(pfn)) { > + if (xen_get_dma_ops(hwdev)->sync_single_for_cpu) > + xen_get_dma_ops(hwdev)->sync_single_for_cpu(hwdev, > handle, size, dir); > + } else > + __xen_dma_sync_single_for_cpu(hwdev, handle, size, dir); > +} > + > +static inline void xen_dma_sync_single_for_device(struct device *hwdev, > + dma_addr_t handle, size_t size, enum dma_data_direction dir) > +{ > + unsigned long pfn = PFN_DOWN(handle); > + if (pfn_valid(pfn)) { > + if
Re: [Xen-devel] Fail to boot Dom0 on Xen Arm64 after "dma-mapping: bypass indirect calls for dma-direct"
On Thu, 17 Jan 2019, Christoph Hellwig wrote: > On Thu, Jan 17, 2019 at 11:43:49AM +, Julien Grall wrote: > > Looking at the change for arm64, you will always call dma-direct API. In > > previous Linux version, xen-swiotlb will call dev->archdata.dev_dma_ops (a > > copy of dev->dma_ops before setting Xen DMA ops) if not NULL. Does it mean > > we expect dev->dma_ops to always be NULL and hence using dma-direct API? > > The way I understood the code from inspecting it and sking the > maintainers a few askings is that for DOM0 we always use xen-swiotlb > as the actual dma_map_ops, but then use the functions in page-coherent.h > only to deal with cache maintainance, so it should be safe. Yes, what you wrote is correct, the stuff in include/xen/arm/page-coherent.h is only to deal with cache maintenance, specifically any cache maintenance operations that might be required by map/unmap_page, dma_sync_single_for_cpu/device. It looks like it is safe today to make the dma_direct calls directly, especially considering that on arm64 it looks like the only other option is iommu_dma_ops which has never been used with Xen so far (the IOMMU has not been exposed to guests yet). On arm32 we don't have this problem because dev->dma_ops is always != NULL with MMU enable (required on Xen), right? So the patch looks fine, I only have an optional suggestion to add a check on dma_ops being unset. I'll reply to the patch. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 10/14] argo: implement the notify op
On Fri, Jan 18, 2019 at 3:54 PM Christopher Clark wrote: > > On Fri, Jan 18, 2019 at 1:44 AM Roger Pau Monné wrote: > > > > On Thu, Jan 17, 2019 at 01:44:32PM -0800, Christopher Clark wrote: > > > On Thu, Jan 17, 2019 at 3:12 AM Roger Pau Monné > > > wrote: > > > > > > > > On Wed, Jan 16, 2019 at 10:54:48PM -0800, Christopher Clark wrote: > > > > > On Tue, Jan 15, 2019 at 8:19 AM Roger Pau Monné > > > > > wrote: > > > > > > > > > > > > On Tue, Jan 15, 2019 at 01:27:42AM -0800, Christopher Clark wrote: > New Situation 4 is: > > 4. Message doesn't fit based on the current available space on the ring, > but Xen can't queue up a VIRQ for later because memory allocation to > add an entry for that failed. Don't set NOTIFY. I should add: there's an additional error condition for Situation 4 besides memory allocation failure: the ring could have reached the maximum number of pending space_available signals in its list, which would cause ENOSPC as another possible error value returned there. > > We ought to enable the guest to distinguish Situation 2 from Situation 4 > -- which I think points to keeping the separate flags. > > > So that would leave the following set of flags: > > > > /* Ring is empty. */ > > #define XEN_ARGO_RING_EMPTY (1U << 0) > > /* Ring exists. */ > > #define XEN_ARGO_RING_EXISTS (1U << 1) > > /* > > * Not enough ring space available for the requested size, caller set > > * to receive a notification via VIRQ_ARGO when enough free space > > * might be available. > > */ > > #define XEN_ARGO_RING_NOTIFY (1U << 2) > > /* Requested size exceeds maximum ring message size. */ > > #define XEN_ARGO_RING_EMSGSIZE(1U << 3) > > /* Ring is shared, not unicast. */ > > #define XEN_ARGO_RING_SHARED (1U << 4) > > > > Note that I've also removed the _DATA_F_, I think it's not specially > > helpful, and shorter names are easier to read. > > Ack - done. > > > > > I think the above is clearer and should be able to convey the > > same set of information using one flag less, which is always better > > IMO. That being set I don't know the users of this interface anyway, > > so if you think the original proposal is better I'm not going to > > oppose. > > ok -- let me know your view given the description of Situation 4 above. > I've kept it unchanged for the time being. > > Christopher ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 10/14] argo: implement the notify op
On Fri, Jan 18, 2019 at 1:44 AM Roger Pau Monné wrote: > > On Thu, Jan 17, 2019 at 01:44:32PM -0800, Christopher Clark wrote: > > On Thu, Jan 17, 2019 at 3:12 AM Roger Pau Monné > > wrote: > > > > > > On Wed, Jan 16, 2019 at 10:54:48PM -0800, Christopher Clark wrote: > > > > On Tue, Jan 15, 2019 at 8:19 AM Roger Pau Monné > > > > wrote: > > > > > > > > > > On Tue, Jan 15, 2019 at 01:27:42AM -0800, Christopher Clark wrote: > > > > > > diff --git a/xen/include/public/argo.h b/xen/include/public/argo.h > > > > > > index c12a50f..d2cb594 100644 > > > > > > --- a/xen/include/public/argo.h > > > > > > +++ b/xen/include/public/argo.h > > > > > > @@ -123,6 +123,42 @@ typedef struct xen_argo_unregister_ring > > > > > > /* Messages on the ring are padded to a multiple of this size. */ > > > > > > #define XEN_ARGO_MSG_SLOT_SIZE 0x10 > > > > > > > > > > > > +/* > > > > > > + * Notify flags > > > > > > + */ > > > > > > +/* Ring is empty */ > > > > > > +#define XEN_ARGO_RING_DATA_F_EMPTY (1U << 0) > > > > > > +/* Ring exists */ > > > > > > +#define XEN_ARGO_RING_DATA_F_EXISTS (1U << 1) > > > > > > +/* Pending interrupt exists. Do not rely on this field - for > > > > > > profiling only */ > > > > > > +#define XEN_ARGO_RING_DATA_F_PENDING (1U << 2) > > > > > > Regarding this flag, I've just noticed while looking at the code that > > > it doesn't seem to relate to interrupts? > > > > It might not seem that way, but I think it does, because it indicates > > that the hypervisor has just queued up a signal (via VIRQ) for later: > > the logic in fill_ring_data has observed that there wasn't enough > > space available in the ring for the requested space_required supplied > > in the notify call, so it has added a new entry to the ring's > > pending_ent list, which will cause a signal to be triggered to the > > domain (ie. a VIRQ) later when enough space has been observed as being > > available. > > Oh, I think I was getting confused by the wording of the comment, here > "pending interrupt" means that the caller should expect an interrupt at > some point in the future when there's enough free space on the ring? Yes, that's right. > To me "pending interrupt" means there's an interrupt set by the > hypervisor which has not yet been serviced by the caller. OK, I could see that is a reasonable interpretation too. Do you have a term that you would prefer for this? > > > Now, the "len" value stored in that pending_ent can be changed later, > > depending on the size of messages that the domain attempts to send to > > the same ring in the meantime, which I think is why the comment notes > > not to depend upon that flag. > > > > > From it's usage in fill_ring_data I would write the following > > > description: > > > > > > "Likely not enough space to queue a message of `space_required` > > > size." > > > > > > And then XEN_ARGO_RING_DATA_F_PENDING is completely orthogonal to > > > XEN_ARGO_RING_DATA_F_SUFFICIENT, at which point having only one of > > > those would be enough? > > > > Given the above, where I do think that the PENDING flag is an > > indicator of queued interrupt, I think there's some merit to keeping > > them separate, rather than committing to the client that it is always > > one or the other. It actually looks like the call to pending_requeue > > is ignoring the potential for an error value (eg ENOSPC or ENOMEM) > > there, where the flag should not be set, and possibly the errno should > > be returned to the caller. > > Yes, you should propagate the errors from pending_requeue to the > caller. ack, done. > > > > AFAICT you cannot get a xen_argo_ring_data_ent_t with both > > > XEN_ARGO_RING_DATA_F_PENDING and XEN_ARGO_RING_DATA_F_SUFFICIENT set > > > at the same time? > > > > right, but there is a case where you can get one with neither bit set. > > Yes, that's right. But you would then get the > XEN_ARGO_RING_DATA_F_EMSGSIZE flag set or the ring simply don't > exist. > > > It looks a bit clearer for the caller to have the explicit separate > > bits because it can avoid having to check a third flag first to see > > how to interpret a combined one. > > There are three possible situations, which are mutually exclusive: > > 1. Message is bigger than the max message size supported by the ring: >set EMSGSIZE > 2. Message fits based on the current available space on the ring: >don't set any flags. > 3. Message doesn't fit based on the current available space on the >ring: set NOTIFY. Unfortunately, given the new error checking (added for my "ack, done." above), now there is a fourth condition. Situation 3 is described more fully as: 3. Message doesn't fit based on the current available space on the ring, and a VIRQ is queued for when space is available: set NOTIFY. New Situation 4 is: 4. Message doesn't fit based on the current available space on the ring, but Xen can't queue up a VIRQ for later because memory allocation to add an entry for that failed. Don't set NOTIFY. We ought to
Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL
On Fri, 18 Jan 2019, Jan Beulich wrote: > >>> On 18.01.19 at 02:24, wrote: > > On Thu, 17 Jan 2019, Jan Beulich wrote: > >> >>> On 17.01.19 at 01:37, wrote: > >> > On Wed, 16 Jan 2019, Jan Beulich wrote: > >> >> In any event - since intermediate variables merely hide the > >> >> casting from the compiler, but they don't remove the casts, the > >> >> solution involving casts is better imo, for incurring less overhead. > >> > > >> > This is where I completely disagree. The intermediate variables are not > >> > hiding casts from the compiler. There were never any pointers in this > >> > case. The linker creates "symbols", not pointers, completely invisible > >> > from C land. Assembly uses these symbols to initialize variables. We > >> > expose these assembly variables as integer to C lands. LD scripts and > >> > assembly have their own terminology and rules: neither "_start" nor > >> > "start" are pointers at any point in time. The operations done in var.S > >> > is not a cast. The C spec is happy, the compiler is happy, MISRA-C is > >> > happy. And we get to avoid the ugly SYMBOL macro that Linux uses. It is > >> > really a win-win. > >> > >> Well, that's a position one can take. But we have to settle on another > >> aspect then first: Does what is not done in C underly C's rules? I > >> thought you were of the opinion that what comes from linker scripts > >> does. In which case what comes from assembly files ought to, too. > >> (FAOD my implication is: If the answer is yes, both approaches > >> violate C's rules. If the answer is no, no change is needed at all.) > > > > Great question, that is the core of the issue. Also, let me premise that > > I agree on the comments you made on the patches (I dislike "start_" > > too), and I can address them if we agree to continue down this path. > > > > But no, I do not think that what is done outside of C-land should follow > > C rules. But I do not agree with your conclusion that in that case there > > is no difference between the approaches. Let's get more into the > > details. > > > > > > 1) SYMBOL_HIDE returning pointer type > > > > Let's take _start and _end as an example. _start is born as a linker > > symbol, and it becomes a C pointer when we do: > > > > extern char _start[], _end[] > > > > Now it is a pointer (actually I should say an array, but let's pretend > > they are the same thing for this discussion). > > > > When we do: > > > > SYMBOL_HIDE(_end) - SYMBOL_HIDE(_start) > > > > We are still subtracting pointers: the pointers returned by SYMBOL_HIDE. > > We cannot prove that they are pointers to the same object or subsequence > > objects in memory, so it is undefined behavior, which is not allowed. > > Stop. No. We very much can prove they are - _end points at > one past the last element of _start[]. It is the compiler which > can't prove the opposite, and hence it can't leverage > undefined behavior for optimization purposes. This is an interesting comment. However, even for normal pointers it is unreliable to count on one pointing one past the last element of the other. This was well explained in the GCC thread linked earlier in this thread. The vision of at least one of the GCC maintainers is that the compiler is free to place things in memory where it wishes, so as a programmer you cannot count on pointers pointing one past the last element of the other. Ever. In this case, where _start and _end are defined outside of C-land, I think it is even more true, and it remains undefined. Moreover, I went back to MISRAC (finally I have a copy) and rule 18.2 says: "subtraction between pointers shall only be applied to pointers that address elements of the same array". So, all the evidence we have seems to say that we cannot rely on _end pointing one past the last element of _start in this matter. > > 3) var.S + start_ as unsigned long > > > > With this approach, _start is born as a linker symbol. It is never > > exported to C, so from C point of view, it doesn't exist. There is > > another variable named "start_" defined in assembly and initialized to > > _start. Now we go into C land with: > > > > extern uintptr_t start_, end_ > > > > start_ and end_ are uintptr_t from the beginning from C point of view. > > They have never been pointers or in any way connected to _start. They > > are "clean". > > > > When we do: > > > > _end - _start > > > > it is a subtraction between uintptr_t, which is allowed. When we do: > > > > for ( call = (const initcall_t *)initcall_start_; > > (uintptr_t)call < presmp_initcall_end_; > > > > The comparison is still between uintptr_t types, and the value of "call" > > still comes from an unsigned long initially. There is never a comparison > > between dubious pointers. (Interger to pointer conversions and pointer > > to integer conversions are allowed by MISRA with some limitations, but I > > am double-checking.) Even: > > > >(uintptr_t)random_pointer < presmp_initcall_end_ > >
Re: [Xen-devel] Unable to boot Linux with master EDK2
On Fri, 18 Jan 2019 at 19:39, Ard Biesheuvel wrote: > > On Fri, 18 Jan 2019 at 19:30, Julien Grall wrote: > > > > Hi all, > > > > I am trying to boot a Xen guest using the latest EDK2 master (cce9d76358 > > "BaseTools: Allow empty value for HiiPcd in Dsc"), GRUB and Linux 5.0-rc2. > > > > The last code executed by Linux is when installing the virtual address > > map in the EFI stub and then it seems to get stuck. I don't have much > > information from the console: > > > > InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7E041040 > > Loading driver at 0x00068C7 EntryPoint=0x00069D65664 > > Loading driver at 0x00068C7 EntryPoint=0x00069D65664 > > InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 7DF6AB18 > > ProtectUefiImageCommon - 0x7E041040 > > - 0x68C7 - 0x02006000 > > SetUefiImageMemoryAttributes - 0x68C7 - 0x1000 > > (0x4008) > > SetUefiImageMemoryAttributes - 0x68C71000 - 0x011CD000 > > (0x00020008) > > SetUefiImageMemoryAttributes - 0x69E3E000 - 0x00E38000 > > (0x4008) > > EFI stub: Booting Linux Kernel... > > EFI stub: Using DTB from configuration table > > EFI stub: Exiting boot services and installing virtual address map... > > XenBus: Set state to 5 > > XenBus: Set state to 5, done > > XenPvBlk: waiting backend state 5, current: 4 > > XenStore: Watch event 7E957398 > > XenBus: Set state to 6 > > XenBus: Set state to 6, done > > XenPvBlk: waiting backend state 6, current: 5 > > XenStore: Watch event 7E957398 > > XenBus: Set state to 1 > > XenBus: Set state to 1, done > > Xen GrantTable, removing 38003 > > Xen GrantTable, removing 38002 > > Xen GrantTable, removing 38001 > > Xen GrantTable, removing 38000 > > SetUefiImageMemoryAttributes - 0x7F36 - 0x0004 > > (0x0008) > > SetUefiImageMemoryAttributes - 0x7BFF - 0x0004 > > (0x0008) > > SetUefiImageMemoryAttributes - 0x7BFA - 0x0004 > > (0x0008) > > SetUefiImageMemoryAttributes - 0x7BF0 - 0x0004 > > (0x0008) > > SetUefiImageMemoryAttributes - 0x7BE6 - 0x0004 > > (0x0008) > > SetUefiImageMemoryAttributes - 0x7BDC - 0x0004 > > (0x0008) > > > > The bisector pointed to the following commit: > > > > commit 2f4a5a9f4c17ed88aaa3114d1e161e42cb80a9bf > > Author: Dandan Bi > > Date: Thu Jan 3 15:31:23 2019 +0800 > > > > MdePkg/BasePeCoffLib: Add more check for relocation data > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1426 > > > > V2: > > (1) Add NULL pointer check for the input parameters > > (2) Add check for the "Adjust" value before applying fix ups. > > > > In function PeCoffLoaderRelocateImageForRuntime, it doesn't > > do much check when do relocation. For API level consideration, > > it's not safe enough. > > So this patch is to replace the same code logic with function > > PeCoffLoaderImageAddress which will cover more validation. > > > > Cc: Michael D Kinney > > Cc: Liming Gao > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Dandan Bi > > Reviewed-by: Liming Gao > > > > Any ideas what could have gone wrong? > > Yes, that patch broke lots of platforms: OVMF, ArmVirtQemu and ARM Juno as well. You need the following patch to fix it https://lists.01.org/pipermail/edk2-devel/2019-January/035372.html ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Unable to boot Linux with master EDK2
On Fri, 18 Jan 2019 at 19:30, Julien Grall wrote: > > Hi all, > > I am trying to boot a Xen guest using the latest EDK2 master (cce9d76358 > "BaseTools: Allow empty value for HiiPcd in Dsc"), GRUB and Linux 5.0-rc2. > > The last code executed by Linux is when installing the virtual address > map in the EFI stub and then it seems to get stuck. I don't have much > information from the console: > > InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7E041040 > Loading driver at 0x00068C7 EntryPoint=0x00069D65664 > Loading driver at 0x00068C7 EntryPoint=0x00069D65664 > InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 7DF6AB18 > ProtectUefiImageCommon - 0x7E041040 > - 0x68C7 - 0x02006000 > SetUefiImageMemoryAttributes - 0x68C7 - 0x1000 > (0x4008) > SetUefiImageMemoryAttributes - 0x68C71000 - 0x011CD000 > (0x00020008) > SetUefiImageMemoryAttributes - 0x69E3E000 - 0x00E38000 > (0x4008) > EFI stub: Booting Linux Kernel... > EFI stub: Using DTB from configuration table > EFI stub: Exiting boot services and installing virtual address map... > XenBus: Set state to 5 > XenBus: Set state to 5, done > XenPvBlk: waiting backend state 5, current: 4 > XenStore: Watch event 7E957398 > XenBus: Set state to 6 > XenBus: Set state to 6, done > XenPvBlk: waiting backend state 6, current: 5 > XenStore: Watch event 7E957398 > XenBus: Set state to 1 > XenBus: Set state to 1, done > Xen GrantTable, removing 38003 > Xen GrantTable, removing 38002 > Xen GrantTable, removing 38001 > Xen GrantTable, removing 38000 > SetUefiImageMemoryAttributes - 0x7F36 - 0x0004 > (0x0008) > SetUefiImageMemoryAttributes - 0x7BFF - 0x0004 > (0x0008) > SetUefiImageMemoryAttributes - 0x7BFA - 0x0004 > (0x0008) > SetUefiImageMemoryAttributes - 0x7BF0 - 0x0004 > (0x0008) > SetUefiImageMemoryAttributes - 0x7BE6 - 0x0004 > (0x0008) > SetUefiImageMemoryAttributes - 0x7BDC - 0x0004 > (0x0008) > > The bisector pointed to the following commit: > > commit 2f4a5a9f4c17ed88aaa3114d1e161e42cb80a9bf > Author: Dandan Bi > Date: Thu Jan 3 15:31:23 2019 +0800 > > MdePkg/BasePeCoffLib: Add more check for relocation data > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1426 > > V2: > (1) Add NULL pointer check for the input parameters > (2) Add check for the "Adjust" value before applying fix ups. > > In function PeCoffLoaderRelocateImageForRuntime, it doesn't > do much check when do relocation. For API level consideration, > it's not safe enough. > So this patch is to replace the same code logic with function > PeCoffLoaderImageAddress which will cover more validation. > > Cc: Michael D Kinney > Cc: Liming Gao > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Dandan Bi > Reviewed-by: Liming Gao > > Any ideas what could have gone wrong? > > Best regards, > > -- > Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Unable to boot Linux with master EDK2
Hi all, I am trying to boot a Xen guest using the latest EDK2 master (cce9d76358 "BaseTools: Allow empty value for HiiPcd in Dsc"), GRUB and Linux 5.0-rc2. The last code executed by Linux is when installing the virtual address map in the EFI stub and then it seems to get stuck. I don't have much information from the console: InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7E041040 Loading driver at 0x00068C7 EntryPoint=0x00069D65664 Loading driver at 0x00068C7 EntryPoint=0x00069D65664 InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 7DF6AB18 ProtectUefiImageCommon - 0x7E041040 - 0x68C7 - 0x02006000 SetUefiImageMemoryAttributes - 0x68C7 - 0x1000 (0x4008) SetUefiImageMemoryAttributes - 0x68C71000 - 0x011CD000 (0x00020008) SetUefiImageMemoryAttributes - 0x69E3E000 - 0x00E38000 (0x4008) EFI stub: Booting Linux Kernel... EFI stub: Using DTB from configuration table EFI stub: Exiting boot services and installing virtual address map... XenBus: Set state to 5 XenBus: Set state to 5, done XenPvBlk: waiting backend state 5, current: 4 XenStore: Watch event 7E957398 XenBus: Set state to 6 XenBus: Set state to 6, done XenPvBlk: waiting backend state 6, current: 5 XenStore: Watch event 7E957398 XenBus: Set state to 1 XenBus: Set state to 1, done Xen GrantTable, removing 38003 Xen GrantTable, removing 38002 Xen GrantTable, removing 38001 Xen GrantTable, removing 38000 SetUefiImageMemoryAttributes - 0x7F36 - 0x0004 (0x0008) SetUefiImageMemoryAttributes - 0x7BFF - 0x0004 (0x0008) SetUefiImageMemoryAttributes - 0x7BFA - 0x0004 (0x0008) SetUefiImageMemoryAttributes - 0x7BF0 - 0x0004 (0x0008) SetUefiImageMemoryAttributes - 0x7BE6 - 0x0004 (0x0008) SetUefiImageMemoryAttributes - 0x7BDC - 0x0004 (0x0008) The bisector pointed to the following commit: commit 2f4a5a9f4c17ed88aaa3114d1e161e42cb80a9bf Author: Dandan Bi Date: Thu Jan 3 15:31:23 2019 +0800 MdePkg/BasePeCoffLib: Add more check for relocation data REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1426 V2: (1) Add NULL pointer check for the input parameters (2) Add check for the "Adjust" value before applying fix ups. In function PeCoffLoaderRelocateImageForRuntime, it doesn't do much check when do relocation. For API level consideration, it's not safe enough. So this patch is to replace the same code logic with function PeCoffLoaderImageAddress which will cover more validation. Cc: Michael D Kinney Cc: Liming Gao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Dandan Bi Reviewed-by: Liming Gao Any ideas what could have gone wrong? Best regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 07/11] optee: add support for RPC SHM buffers
Hi Julien, Julien Grall writes: > Hi Volodymyr, > > On 18/01/2019 16:05, Volodymyr Babchuk wrote: >> Julien Grall writes: > If you happen to make MAX_STD_CALLS dynamic, then this should also be > dynamic. Of course. [...] +static void handle_rpc_func_alloc(struct optee_domain *ctx, + struct cpu_user_regs *regs) +{ +paddr_t ptr = get_user_reg(regs, 1) << 32 | get_user_reg(regs, 2); + +if ( ptr & (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1) ) +gprintk(XENLOG_WARNING, "Domain returned invalid RPC command buffer\n"); >>> >>> Should not you bail-out in that case? Also, I would turn it to a >>> gdprintk. >> OP-TEE does own checks and that check will fail also. Then OP-TEE will >> issue request to free this SHM. > > I think it is better if we go on the safe-side. I.e if we know there > would be an error (like here), then you need to return an error in > from Xen rather than calling OP-TEE. Otherwise, you may end up to > nasty problem. Actually, I don't see how I can do this. This is an RPC response. I can't return error to RPC response. All I can do is to mangle RPC response in a such way, that OP-TEE surely will treat it as an error and act accordingly. >>> >>> I am not sure to understand what you mean here. Surely if the address >>> is not aligned, then OP-TEE will return an error too, right? So can't >>> you emulate OP-TEE behavior in that case? >> No, OP-TEE will not return an error. OP-TEE will handle it as an error. > > Will it? Yes. It checks for page alignment in the same way, as Xen does. > Looking at the code again, you will pass either 0 (when not > able to translate the address) or page_to_maddr(...) value. So the > value will get truncated by Xen with just a warning. > > Is it the expected behavior? No, this is a bug and I'll fix it. >> For OP-TEE any RPC looks like a function call. So it excepts some return >> value - buffer pointer in this case. If it gets invalid pointer, it >> issues another RPC to free this buffer and then propagates error back to >> caller. >> >> So, if I'll return error back to the guest (or rather issue RPC to free >> the buffer), OP-TEE will be still blocked, waiting for a pointer from >> the guest. Of course I can go further end emulate a new buffer >> allocation RPC but from Xen side in a hope that guest will provide valid >> buffer this time. But, what if not? And why I should decide instead of >> OP-TEE? > > My main concern here is you do a print a warning and continue like > nothing happen. As a reviewer, this is worrying because it is a call > for weird behavior to happen in Xen. We don't want that. Yep, I made a mistake there. I need to pass invalid value to OP-TEE to make sure that it will not mistake it with semi-valid address. > Looking at the code, why can't you pass 0 as you do if you fail to pin the > pages? This is exactly what I'm going to do. -- Best regards, Volodymyr Babchuk ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [GIT PULL] xen: fixes for 5.0-rc3
The pull request you sent on Fri, 18 Jan 2019 10:57:13 +0100: > git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git > for-linus-5.0-rc3-tag has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/e6ec2fda2d464938989ecd770be92e492ace3ae1 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen-unstable PVHdom0: Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at iommu.c:324
On Fri, Jan 18, 2019 at 03:17:57PM +0100, Sander Eikelenboom wrote: > On 18/01/2019 13:50, Roger Pau Monné wrote: > > On Fri, Jan 18, 2019 at 01:03:04PM +0100, Sander Eikelenboom wrote: > >> Hi Roger, > >> > >> I gave PVH dom0 a spin, see how far I would get. > > > > Thanks! > > > >> With current xen-unstable unfortunately not that far, i got the splat > >> below. > > > > Yes, this was already reported: > > > > https://lists.xenproject.org/archives/html/xen-devel/2019-01/msg01030.html > >> If you need more info, would like me to test a patch (or some other git > >> tree/branch), > >> I will be happy to give it a spin ! > > > > Paul is working on a fix, but in the meantime just removing the > > assertions should be fine: > > > > ---8<--- > > diff --git a/xen/drivers/passthrough/iommu.c > > b/xen/drivers/passthrough/iommu.c > > index bd1af35a13..98e6fc35e2 100644 > > --- a/xen/drivers/passthrough/iommu.c > > +++ b/xen/drivers/passthrough/iommu.c > > @@ -321,9 +321,6 @@ int iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn, > > if ( !iommu_enabled || !hd->platform_ops ) > > return 0; > > > > -ASSERT(IS_ALIGNED(dfn_x(dfn), (1ul << page_order))); > > -ASSERT(IS_ALIGNED(mfn_x(mfn), (1ul << page_order))); > > - > > for ( i = 0; i < (1ul << page_order); i++ ) > > { > > rc = hd->platform_ops->map_page(d, dfn_add(dfn, i), mfn_add(mfn, > > i), > > > > I gave that a spin and i now get a seemingly endless stream of IO_PAGE_FAULTs You shouldn't get those page faults since they are for addresses that belong to a reserved region, and that should be mapped into the p2m. I've just tested on my AMD box and I'm also seeing errors (albeit different ones), so I guess something broke since I last fixed PVH Dom0 to boot on AMD hardware. I've also tested commit: commit fad6ba64a8c98bebb9374f390cc255fac05237ab (HEAD) Author: Roger Pau Monné Date: Fri Nov 30 12:10:00 2018 +0100 amd/iommu: skip host bridge devices when updating IOMMU page tables And it works on my AMD box and I'm able to boot as a PVH Dom0. Can you give this commit a spin? Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-3.18 test] 131990: regressions - FAIL
flight 131990 linux-3.18 real [real] http://logs.test-lab.xenproject.org/osstest/logs/131990/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-examine 8 reboot fail REGR. vs. 128858 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-boot fail REGR. vs. 128858 test-amd64-i386-xl-shadow 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-xl-qemuu-debianhvm-amd64 7 xen-bootfail REGR. vs. 128858 test-amd64-amd64-xl-multivcpu 7 xen-bootfail REGR. vs. 128858 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-qemuu-nested-intel 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 128858 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 128858 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 128858 test-amd64-i386-xl7 xen-boot fail REGR. vs. 128858 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 128858 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 128858 test-amd64-amd64-xl 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-xl-qemuu-ovmf-amd64 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-amd64-pvgrub 7 xen-bootfail REGR. vs. 128858 test-amd64-amd64-rumprun-amd64 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-i386-pvgrub 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-libvirt-pair 10 xen-boot/src_host fail REGR. vs. 128858 test-amd64-amd64-libvirt-pair 11 xen-boot/dst_host fail REGR. vs. 128858 test-amd64-amd64-xl-credit2 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-pair10 xen-boot/src_hostfail REGR. vs. 128858 test-amd64-amd64-xl-pvshim7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-pair11 xen-boot/dst_hostfail REGR. vs. 128858 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-xl-xsm 7 xen-boot fail REGR. vs. 128858 test-amd64-i386-freebsd10-i386 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-libvirt-xsm 7 xen-boot fail REGR. vs. 128858 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 128858 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-boot fail REGR. vs. 128858 test-amd64-i386-freebsd10-amd64 7 xen-boot fail REGR. vs. 128858 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 128858 Tests which are failing intermittently (not blocking): test-amd64-i386-xl-qemuu-debianhvm-amd64 16 guest-localmigrate/x10 fail in 131969 pass in 131990 test-armhf-armhf-xl-credit2 10 debian-install fail pass in 131969 test-armhf-armhf-libvirt 16 guest-start/debian.repeat fail pass in 131969 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 7 xen-boot fail REGR. vs. 128858 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-raw 10 debian-di-install fail in 131969 like 128841 test-armhf-armhf-xl-credit2 13 migrate-support-check fail in 131969 never pass test-armhf-armhf-xl-credit2 14 saverestore-support-check fail in 131969 never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 128858 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 128858 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 128858 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 16 guest-localmigrate/x10 fail like 128858 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 128858 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 128858 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 128858 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17
Re: [Xen-devel] [PATCH v3 07/11] optee: add support for RPC SHM buffers
Hi Volodymyr, On 18/01/2019 16:05, Volodymyr Babchuk wrote: Julien Grall writes: If you happen to make MAX_STD_CALLS dynamic, then this should also be dynamic. Of course. [...] +static void handle_rpc_func_alloc(struct optee_domain *ctx, + struct cpu_user_regs *regs) +{ +paddr_t ptr = get_user_reg(regs, 1) << 32 | get_user_reg(regs, 2); + +if ( ptr & (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1) ) +gprintk(XENLOG_WARNING, "Domain returned invalid RPC command buffer\n"); Should not you bail-out in that case? Also, I would turn it to a gdprintk. OP-TEE does own checks and that check will fail also. Then OP-TEE will issue request to free this SHM. I think it is better if we go on the safe-side. I.e if we know there would be an error (like here), then you need to return an error in from Xen rather than calling OP-TEE. Otherwise, you may end up to nasty problem. Actually, I don't see how I can do this. This is an RPC response. I can't return error to RPC response. All I can do is to mangle RPC response in a such way, that OP-TEE surely will treat it as an error and act accordingly. I am not sure to understand what you mean here. Surely if the address is not aligned, then OP-TEE will return an error too, right? So can't you emulate OP-TEE behavior in that case? No, OP-TEE will not return an error. OP-TEE will handle it as an error. Will it? Looking at the code again, you will pass either 0 (when not able to translate the address) or page_to_maddr(...) value. So the value will get truncated by Xen with just a warning. Is it the expected behavior? For OP-TEE any RPC looks like a function call. So it excepts some return value - buffer pointer in this case. If it gets invalid pointer, it issues another RPC to free this buffer and then propagates error back to caller. So, if I'll return error back to the guest (or rather issue RPC to free the buffer), OP-TEE will be still blocked, waiting for a pointer from the guest. Of course I can go further end emulate a new buffer allocation RPC but from Xen side in a hope that guest will provide valid buffer this time. But, what if not? And why I should decide instead of OP-TEE? My main concern here is you do a print a warning and continue like nothing happen. As a reviewer, this is worrying because it is a call for weird behavior to happen in Xen. We don't want that. Looking at the code, why can't you pass 0 as you do if you fail to pin the pages? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] iommu: specify page_count rather than page_order to iommu_map/unmap()...
On 18/01/2019 16:03, Paul Durrant wrote: > diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h > index cdc8021cbd..82fb86c7ff 100644 > --- a/xen/include/xen/iommu.h > +++ b/xen/include/xen/iommu.h > @@ -111,17 +111,17 @@ enum > #define IOMMU_FLUSHF_modified (1u << _IOMMU_FLUSHF_modified) > > int __must_check iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn, > - unsigned int page_order, unsigned int flags, > + unsigned int page_count, unsigned int flags, > unsigned int *flush_flags); I'd take the opportunity to make page_count an unsigned long, as we can now sensibly issue a single call for an entire BAR, and some graphics card BARs are getting to be a ludicrous size. Otherwise, LGTM and can also be fixed on commit. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 132059: tolerable all pass - PUSHED
flight 132059 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/132059/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 1912f1220cf87aee28349469893f101980714a05 baseline version: xen 50923ade7a3d1a87b3a46020daa2ed897eb749e3 Last test of basis 132022 2019-01-17 19:00:39 Z0 days Testing same since 132059 2019-01-18 15:01:12 Z0 days1 attempts People who touched revisions under test: Ian Jackson Wei Liu jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass 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 50923ade7a..1912f1220c 1912f1220cf87aee28349469893f101980714a05 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.9 test] 131991: regressions - FAIL
flight 131991 linux-4.9 real [real] http://logs.test-lab.xenproject.org/osstest/logs/131991/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-examine 4 memdisk-try-append fail REGR. vs. 131971 test-amd64-amd64-xl-qcow211 guest-start fail REGR. vs. 131971 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 131971 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 131971 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 131971 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 131971 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 131971 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: linux008bfb9312968bc6af54e47746a9d9f66c8388c0 baseline version: linuxdf6062688e387419f0e10ee1bef2e9cfd7795399 Last test of basis 131971 2019-01-15 23:03:47 Z2 days Testing same since 131991 2019-01-17 05:04:56 Z1 days1 attempts People who touched revisions under test: Alan Stern Andrew Morton Christoph Lameter Daniele Palmas Dongsheng Yang Greg Kroah-Hartman Hans de Goede Icenowy Zheng Ilya Dryomov J. Bruce Fields Jack Stocker Jan Stancek Kailang Yang Kirill A. Shutemov Linus Torvalds Michal Hocko Pavel Shilovsky Rafael J. Wysocki Ross Lagerwall Steve French Takashi Iwai Theodore Ts'o Vasily Averin Wolfram Sang Yi Zeng jobs: build-amd64-xsm pass
[Xen-devel] osstest stretch upgrade, progress/info/todo
We talked on irc about this, which is needed for deploying the two Thunder-X boxes. I have pushed my current working branch here: xenbits.xen.org:/home/iwj/ext/osstest.git#wip.stretch I have been keeping my todo list in a Trello card. I have c the most important parts here: Checklist - part I X filter branch into wanted for test and not wanted for test X run test flight X collect results and write items X investigate rochester* X sort out network naming situation X rochester kernel hdd driver X arm64 rochester1 install failure ? check for ts-kernel-build: disable components that don't build test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm seems like xvda missing maybe ? Needs investigation refactor ts-guests-nbd-mirror: use target_{get,put}file_root to transfter cfg X ed25519 host keys in standard set (will hopefully fix migration) above this, retest/setup/etc. in progress cambridge d-i clear checklist ticks and go back to "run test flight" Checklist - part II tidy branch review everything send series publicly await acks etc. push all but "switch to stretch" push "switch to stretch" Deferred until after push to master investigate joubertin* investigate merlot* (X means the item was marked done in my Trello.) And I have c the "activity" section which includes comments which I often use to record runes I have been using. Ian. Activity Hide Details Ian Jackson (Citrix) Nov 30, 2018 at 4:52 PM (edited) guests-nbd-mirror refactoring OSSTEST_JOBS_ONLY=test-amd64-amd64-pair ./make-flight osstest xen-unstable adhoc 130861 130887 ./mg-execute-flight -Badhoc -eian.jack...@citrix.com -P $flight 4testing.git 130888 Edit - Delete Ian Jackson (Citrix) completed ed25519 host keys in standard set (will hopefully fix migration) on this card Nov 30, 2018 at 4:20 PM Ian Jackson (Citrix) completed arm64 rochester1 install failure ? on this card Nov 30, 2018 at 4:15 PM Ian Jackson (Citrix) Nov 30, 2018 at 4:08 PM ./mg-repro-setup 130801 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm debian-hvm-install alloc:'{blessed-real,arch-amd64}' 3testing.git 130886 Edit - Delete Ian Jackson (Citrix) Nov 30, 2018 at 3:44 PM (edited) Revert "XXX ts-kernel-build: disable components that don't build" 130880 ./cs-adjust-flight new:adhoc copy-jobs 130861 ^build basis=130801 ./mg-execute-flight -f$basis -P -eian.jack...@citrix.com $flight 2testing.git Edit - Delete ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] iommu: specify page_count rather than page_order to iommu_map/unmap()...
> -Original Message- > From: Julien Grall [mailto:julien.gr...@arm.com] > Sent: 18 January 2019 16:07 > To: Paul Durrant ; xen-devel@lists.xenproject.org > Cc: Chao Gao ; Sander Eikelenboom > ; Jan Beulich ; Andrew Cooper > ; Wei Liu ; Roger Pau > Monne ; George Dunlap ; > Ian Jackson ; Konrad Rzeszutek Wilk > ; Stefano Stabellini ; Tim > (Xen.org) ; Jun Nakajima ; Kevin Tian > > Subject: Re: [PATCH] iommu: specify page_count rather than page_order to > iommu_map/unmap()... > > > > On 18/01/2019 16:03, Paul Durrant wrote: > > ...and remove alignment assertions. > > > > Testing shows that certain callers of iommu_legacy_map/unmap() specify > > order > 0 ranges that are not order aligned thus causing one of the > > IS_ALIGNED() assertions to fire. > > > > This patch removes those assertions and modifies iommu_map/unmap() and > > iommu_legacy_map/unmap() to take a page_count argument rather than a > > page_order. Using a count actually makes more sense because the valid > > set of mapping orders is specific to the IOMMU implementation and to it > > should be up to the implementation specific code to translate a mapping > > count into an optimal set of mapping orders (when the code is finally > > modified to support orders > 0). > > > > Signed-off-by: Paul Durrant > > --- > > Reported-by: Chao Gao > > Reported-by: Sander Eikelenboom > You put those tags after ---. Don't you want them in the final commit? > Good point. If there is a v2 then I'll move them, otherwise I hope that can be fixed up on commit. Paul > Cheers, > > -- > Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] iommu: specify page_count rather than page_order to iommu_map/unmap()...
On 18/01/2019 16:03, Paul Durrant wrote: ...and remove alignment assertions. Testing shows that certain callers of iommu_legacy_map/unmap() specify order > 0 ranges that are not order aligned thus causing one of the IS_ALIGNED() assertions to fire. This patch removes those assertions and modifies iommu_map/unmap() and iommu_legacy_map/unmap() to take a page_count argument rather than a page_order. Using a count actually makes more sense because the valid set of mapping orders is specific to the IOMMU implementation and to it should be up to the implementation specific code to translate a mapping count into an optimal set of mapping orders (when the code is finally modified to support orders > 0). Signed-off-by: Paul Durrant --- Reported-by: Chao Gao Reported-by: Sander Eikelenboom You put those tags after ---. Don't you want them in the final commit? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 07/11] optee: add support for RPC SHM buffers
Hi Julien, Julien Grall writes: [...] >> diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c >> index dc90e2ed8e..771148e940 100644 >> --- a/xen/arch/arm/tee/optee.c >> +++ b/xen/arch/arm/tee/optee.c >> @@ -30,6 +30,12 @@ >> * OP-TEE spawns a thread for every standard call. >> */ >> #define MAX_STD_CALLS 16 >> +/* >> + * Maximal number of pre-allocated SHM buffers. OP-TEE generally asks >> + * for one SHM buffer per thread, so this also corresponds to OP-TEE >> + * option CFG_NUM_THREADS >> + */ > > Same as patch #6 regarding CFG_NUM_THREADS. Right now OP-TEE will not allocate more than one buffer per OP-TEE thread. And I can see no reason why it would change. So, basically I can remove this MAX_RPC_SHMS at all and use MAX_STD_CALLS instead. But then it will be not so obvious, why I compare number of SHM buffers with number of std calls. Thus, I think it is good to have separate define and comment. >>> >>> I am not against have the 2 defines, what I was pointed out with the >>> documentation on top is incorrect as patch #6. >> I'm sorry, I don't quite understand. In patch #6 your concern was that >> CFG_NUM_THREADS depends on platform, right? > > My first concern was the documentation does not reflect the reality > because CFG_NUM_THREADS is not always equal to 16. > Ideally we should be able to know the number of threads supported. But > that could be a follow-up patch (or potentially ignored) if nothing > bad can happen when Xen handles more thread than OP-TEE does. No, it is valid situation. Opposite (when OP-TEE can handle more threads than Xen) also can be handled in a right way. > In any case, the documentation in Xen should reflect the reality. Ah, okay, got it. >> >>> If you happen to make MAX_STD_CALLS dynamic, then this should also be >>> dynamic. >> Of course. >> >> [...] >> +static void handle_rpc_func_alloc(struct optee_domain *ctx, >> + struct cpu_user_regs *regs) >> +{ >> +paddr_t ptr = get_user_reg(regs, 1) << 32 | get_user_reg(regs, 2); >> + >> +if ( ptr & (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1) ) >> +gprintk(XENLOG_WARNING, "Domain returned invalid RPC command >> buffer\n"); > > Should not you bail-out in that case? Also, I would turn it to a gdprintk. OP-TEE does own checks and that check will fail also. Then OP-TEE will issue request to free this SHM. >>> >>> I think it is better if we go on the safe-side. I.e if we know there >>> would be an error (like here), then you need to return an error in >>> from Xen rather than calling OP-TEE. Otherwise, you may end up to >>> nasty problem. >> Actually, I don't see how I can do this. This is an RPC response. I >> can't return error to RPC response. All I can do is to mangle RPC >> response in a such way, that OP-TEE surely will treat it as an error and >> act accordingly. > > I am not sure to understand what you mean here. Surely if the address > is not aligned, then OP-TEE will return an error too, right? So can't > you emulate OP-TEE behavior in that case? No, OP-TEE will not return an error. OP-TEE will handle it as an error. For OP-TEE any RPC looks like a function call. So it excepts some return value - buffer pointer in this case. If it gets invalid pointer, it issues another RPC to free this buffer and then propagates error back to caller. So, if I'll return error back to the guest (or rather issue RPC to free the buffer), OP-TEE will be still blocked, waiting for a pointer from the guest. Of course I can go further end emulate a new buffer allocation RPC but from Xen side in a hope that guest will provide valid buffer this time. But, what if not? And why I should decide instead of OP-TEE? > >> >>> Also, I think you want a comment (maybe in the commit message) >>> explaining that OPTEE_MSG_NONCONTIG_PAGE_SIZE will always be equal to >>> PAGE_SIZE. >> It is always equal to 4096 bytes. But, AFAIK, XEN can work with other >> PAGE_SIZEs, isn't? Actually, linux driver support other page sizes, it >> just splits those pages into 4k chunks. The same can be done in XEN, but >> I don't want to introduce this logic right now. The patches are complex >> enough. Right now we have >> BUILD_BUG_ON(PAGE_SIZE != OPTEE_MSG_NONCONTIG_PAGE_SIZE) and this is >> enough, I hope. > > Xen only support 4KB page, but I wouldn't bet that in the future on > Arm as we will require 64KB page for some features (i.e 52-bit > support). So, I was mistaken. For some reason I though that Xen can handle other page sizes as well. > I wasn't asking you to handle a different PAGE_SIZE, just to clarify > in a comment on top that you expect the both defined to be > valid. Another BUILD_BUG_ON is not necessary assuming you add the one > in patch #6. Okay, I'll add such comment. -- Best regards,Volodymyr Babchuk
[Xen-devel] [PATCH] iommu: specify page_count rather than page_order to iommu_map/unmap()...
...and remove alignment assertions. Testing shows that certain callers of iommu_legacy_map/unmap() specify order > 0 ranges that are not order aligned thus causing one of the IS_ALIGNED() assertions to fire. This patch removes those assertions and modifies iommu_map/unmap() and iommu_legacy_map/unmap() to take a page_count argument rather than a page_order. Using a count actually makes more sense because the valid set of mapping orders is specific to the IOMMU implementation and to it should be up to the implementation specific code to translate a mapping count into an optimal set of mapping orders (when the code is finally modified to support orders > 0). Signed-off-by: Paul Durrant --- Reported-by: Chao Gao Reported-by: Sander Eikelenboom Cc: Jan Beulich Cc: Andrew Cooper Cc: Wei Liu Cc: "Roger Pau Monné" Cc: George Dunlap Cc: Ian Jackson Cc: Julien Grall Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Tim Deegan Cc: Jun Nakajima Cc: Kevin Tian Cc: George Dunlap --- xen/arch/x86/mm.c | 6 ++ xen/arch/x86/mm/p2m-ept.c | 5 +++-- xen/arch/x86/mm/p2m-pt.c| 4 ++-- xen/arch/x86/mm/p2m.c | 9 + xen/arch/x86/x86_64/mm.c| 6 ++ xen/common/grant_table.c| 8 xen/drivers/passthrough/iommu.c | 29 +++-- xen/drivers/passthrough/x86/iommu.c | 4 ++-- xen/include/xen/iommu.h | 8 9 files changed, 35 insertions(+), 44 deletions(-) diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 7ec5954b03..caccfe3f79 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -2801,11 +2801,9 @@ static int _get_page_type(struct page_info *page, unsigned long type, mfn_t mfn = page_to_mfn(page); if ( (x & PGT_type_mask) == PGT_writable_page ) -iommu_ret = iommu_legacy_unmap(d, _dfn(mfn_x(mfn)), - PAGE_ORDER_4K); +iommu_ret = iommu_legacy_unmap(d, _dfn(mfn_x(mfn)), 1); else if ( type == PGT_writable_page ) -iommu_ret = iommu_legacy_map(d, _dfn(mfn_x(mfn)), mfn, - PAGE_ORDER_4K, +iommu_ret = iommu_legacy_map(d, _dfn(mfn_x(mfn)), mfn, 1, IOMMUF_readable | IOMMUF_writable); } diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c index 2b2bf31aad..56341ca678 100644 --- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -885,8 +885,9 @@ out: rc = iommu_pte_flush(d, gfn, _entry->epte, order, vtd_pte_present); else if ( need_iommu_pt_sync(d) ) rc = iommu_flags ? -iommu_legacy_map(d, _dfn(gfn), mfn, order, iommu_flags) : -iommu_legacy_unmap(d, _dfn(gfn), order); +iommu_legacy_map(d, _dfn(gfn), mfn, 1u << order, + iommu_flags) : +iommu_legacy_unmap(d, _dfn(gfn), 1u << order); } unmap_domain_page(table); diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c index 12f92cf1f0..ac86a895a0 100644 --- a/xen/arch/x86/mm/p2m-pt.c +++ b/xen/arch/x86/mm/p2m-pt.c @@ -694,9 +694,9 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn, if ( need_iommu_pt_sync(p2m->domain) ) rc = iommu_pte_flags ? -iommu_legacy_map(d, _dfn(gfn), mfn, page_order, +iommu_legacy_map(d, _dfn(gfn), mfn, 1u << page_order, iommu_pte_flags) : -iommu_legacy_unmap(d, _dfn(gfn), page_order); +iommu_legacy_unmap(d, _dfn(gfn), 1u << page_order); else if ( iommu_use_hap_pt(d) && iommu_old_flags ) amd_iommu_flush_pages(p2m->domain, gfn, page_order); } diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index d14ce57dd5..ae3d2acd36 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -780,7 +780,8 @@ p2m_remove_page(struct p2m_domain *p2m, unsigned long gfn_l, unsigned long mfn, if ( !paging_mode_translate(p2m->domain) ) return need_iommu_pt_sync(p2m->domain) ? -iommu_legacy_unmap(p2m->domain, _dfn(mfn), page_order) : 0; +iommu_legacy_unmap(p2m->domain, _dfn(mfn), 1u << page_order) : +0; ASSERT(gfn_locked_by_me(p2m, gfn)); P2M_DEBUG("removing gfn=%#lx mfn=%#lx\n", gfn_l, mfn); @@ -827,7 +828,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn, if ( !paging_mode_translate(d) ) return (need_iommu_pt_sync(d) && t == p2m_ram_rw) ? -iommu_legacy_map(d, _dfn(mfn_x(mfn)), mfn, page_order, +iommu_legacy_map(d, _dfn(mfn_x(mfn)), mfn, 1u << page_order, IOMMUF_readable | IOMMUF_writable) : 0; /* foreign pages
Re: [Xen-devel] [RFC PATCH 2/2] xen: credit2: cached attribute for next runqueue entry
On Sun, 2018-12-23 at 19:51 +0530, Praveen Kumar wrote: > --- a/xen/common/sched_credit2.c > +++ b/xen/common/sched_credit2.c > @@ -473,6 +473,7 @@ struct csched2_runqueue_data { > spinlock_t lock; /* Lock for this > runqueue */ > > struct rb_root runq; /* Runqueue is an > rbtree */ > +struct rb_node *next_elem; /* Cached entry to run > next */ > I don't like the name much. IIRC, in Linux, the field was called 'rb_leftmost'. So we can call it rb_rightmost. Or maybe something like rq_rightmost, or rq_first (or runq_first). And in the comment, I'd say something like, "vcpu with highest credits in the runq" > @@ -1301,6 +1302,7 @@ runq_insert(const struct scheduler *ops, struct > csched2_vcpu *svc) > { > unsigned int cpu = svc->vcpu->processor; > struct rb_root *runq = (ops, cpu)->runq; > +struct csched2_runqueue_data *rqd = svc->rqd; > int pos = 0; > > ASSERT(spin_is_locked(per_cpu(schedule_data, > cpu).schedule_lock)); > @@ -1314,6 +1316,7 @@ runq_insert(const struct scheduler *ops, struct > csched2_vcpu *svc) > ASSERT(!(svc->flags & CSFLAG_scheduled)); > > rb_runq_insert(runq, svc, ); > +rqd->next_elem = rb_last(runq); > Ah, ok, yes, using rb_last() here (and in runq_remove()) is indeed better than doing it int runq_candidate(). Still not what I had in mind, though. In fact, the whole point of this "let's cache the rightmost element" would be to avoid having to call rb_last() pretty much at all. How? Well, basically, inside the while() that you're adding here in runq_insert(), you make a note about whether the new vcpu has been inserted in the righmost spot of the tree. And if it was, you update the cache. Similarly, when removing a vcpu, you check whether you are removing the rightmost element of the tree. And if you are, you update the cache. For instance, see how they've been doing this in CFS (before the rb- tree cached helpers were introduced), when inserting a task in the runqueue: https://elixir.bootlin.com/linux/v3.15/source/kernel/sched/fair.c#L490 and when removing it: https://elixir.bootlin.com/linux/v3.15/source/kernel/sched/fair.c#L526 Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Software Engineer @ SUSE https://www.suse.com/ signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 07/11] optee: add support for RPC SHM buffers
On 17/01/2019 21:01, Volodymyr Babchuk wrote: Hi, Hi Volodymyr, Julien Grall writes: HI, On 1/17/19 7:48 PM, Volodymyr Babchuk wrote: Julien Grall writes: Hi Volodymyr, On 18/12/2018 21:11, Volodymyr Babchuk wrote: From: Volodymyr Babchuk [...] diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c index dc90e2ed8e..771148e940 100644 --- a/xen/arch/arm/tee/optee.c +++ b/xen/arch/arm/tee/optee.c @@ -30,6 +30,12 @@ * OP-TEE spawns a thread for every standard call. */ #define MAX_STD_CALLS 16 +/* + * Maximal number of pre-allocated SHM buffers. OP-TEE generally asks + * for one SHM buffer per thread, so this also corresponds to OP-TEE + * option CFG_NUM_THREADS + */ Same as patch #6 regarding CFG_NUM_THREADS. Right now OP-TEE will not allocate more than one buffer per OP-TEE thread. And I can see no reason why it would change. So, basically I can remove this MAX_RPC_SHMS at all and use MAX_STD_CALLS instead. But then it will be not so obvious, why I compare number of SHM buffers with number of std calls. Thus, I think it is good to have separate define and comment. I am not against have the 2 defines, what I was pointed out with the documentation on top is incorrect as patch #6. I'm sorry, I don't quite understand. In patch #6 your concern was that CFG_NUM_THREADS depends on platform, right? My first concern was the documentation does not reflect the reality because CFG_NUM_THREADS is not always equal to 16. Ideally we should be able to know the number of threads supported. But that could be a follow-up patch (or potentially ignored) if nothing bad can happen when Xen handles more thread than OP-TEE does. In any case, the documentation in Xen should reflect the reality. If you happen to make MAX_STD_CALLS dynamic, then this should also be dynamic. Of course. [...] +static void handle_rpc_func_alloc(struct optee_domain *ctx, + struct cpu_user_regs *regs) +{ +paddr_t ptr = get_user_reg(regs, 1) << 32 | get_user_reg(regs, 2); + +if ( ptr & (OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1) ) +gprintk(XENLOG_WARNING, "Domain returned invalid RPC command buffer\n"); Should not you bail-out in that case? Also, I would turn it to a gdprintk. OP-TEE does own checks and that check will fail also. Then OP-TEE will issue request to free this SHM. I think it is better if we go on the safe-side. I.e if we know there would be an error (like here), then you need to return an error in from Xen rather than calling OP-TEE. Otherwise, you may end up to nasty problem. Actually, I don't see how I can do this. This is an RPC response. I can't return error to RPC response. All I can do is to mangle RPC response in a such way, that OP-TEE surely will treat it as an error and act accordingly. I am not sure to understand what you mean here. Surely if the address is not aligned, then OP-TEE will return an error too, right? So can't you emulate OP-TEE behavior in that case? Also, I think you want a comment (maybe in the commit message) explaining that OPTEE_MSG_NONCONTIG_PAGE_SIZE will always be equal to PAGE_SIZE. It is always equal to 4096 bytes. But, AFAIK, XEN can work with other PAGE_SIZEs, isn't? Actually, linux driver support other page sizes, it just splits those pages into 4k chunks. The same can be done in XEN, but I don't want to introduce this logic right now. The patches are complex enough. Right now we have BUILD_BUG_ON(PAGE_SIZE != OPTEE_MSG_NONCONTIG_PAGE_SIZE) and this is enough, I hope. Xen only support 4KB page, but I wouldn't bet that in the future on Arm as we will require 64KB page for some features (i.e 52-bit support). I wasn't asking you to handle a different PAGE_SIZE, just to clarify in a comment on top that you expect the both defined to be valid. Another BUILD_BUG_ON is not necessary assuming you add the one in patch #6. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH v2 1/2] xen: credit2: rb-tree for runqueues
On Sun, 2018-12-23 at 19:51 +0530, Praveen Kumar wrote: > diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c > index 623a325ceb..2463a25f87 100644 > --- a/xen/common/sched_credit2.c > +++ b/xen/common/sched_credit2.c > @@ -471,7 +472,7 @@ custom_param("credit2_runqueue", > parse_credit2_runqueue); > struct csched2_runqueue_data { > spinlock_t lock; /* Lock for this > runqueue */ > > -struct list_head runq; /* Ordered list of runnable > vms */ > +struct rb_root runq; /* Runqueue is an > rbtree */ > I wouldn't change the comment. It's useful to know that the idea is to use this field to keep a list of runnable vcpus, and that we want it to be ordered, which is what the comment currently says. It's pointless to state that we're using an rb-tree, because once can tell that, by just looking at the data type. Actually, the comment says "Ordered list of runnable vms", while I think it would be better if it said "Ordered list of runnable vcpus". Changing "vms" to "vcpus" does not belong in this patch, strictly speaking, but if you want to do that, I could live with that. If you don't want to do it, then don't, and just leave the comment alone. > @@ -604,6 +605,28 @@ static inline bool has_cap(const struct > csched2_vcpu *svc) > return svc->budget != STIME_MAX; > } > > +static void rb_runq_insert(struct rb_root *root, > + struct csched2_vcpu *svc, > + int *pos) > +{ > +struct csched2_vcpu *entry = NULL; > I'd call this (something like) 'parent_svc'. It makes it easier to understand what's actually happening in the loop below. > +struct rb_node **node = >rb_node; > +struct rb_node *parent = NULL; > + > +while (*node) { > +parent = *node; > +entry = rb_entry(parent, struct csched2_vcpu, runq_elem); > +if ( svc->credit < entry->credit ) > +node = >rb_left; > +else > +node = >rb_right; > + > +(*pos)++; > +} > @@ -1789,6 +1803,7 @@ static void park_vcpu(struct csched2_vcpu *svc) > * In both cases, we also add it to the list of parked vCPUs of > the domain. > */ > __set_bit(_VPF_parked, >pause_flags); > + > if ( vcpu_on_runq(svc) ) > { > runq_remove(svc); > Unrelated change. Don't do it. :-) > @@ -2087,6 +2102,8 @@ csched2_vcpu_sleep(const struct scheduler *ops, > struct vcpu *vc) > } > else if ( vcpu_on_runq(svc) ) > { > +printk(XENLOG_WARNING "%s : %d : vcpu on runq rb !\n", > __func__, __LINE__); > + > So, this, and all the various other lines similar to this are/have been useful for debugging, I guess? That is ok, but I don't think they should land upstream. If you think they (or maybe some of them) should, then make it/them proper debugging output. And, probably, do that in its own patch, as it would not be related to what the data structure used for the runqueues is. But again, IMO, you can get rid of all of them. > ASSERT(svc->rqd == c2rqd(ops, vc->processor)); > update_load(ops, svc->rqd, svc, -1, NOW()); > runq_remove(svc); > @@ -2764,8 +2783,10 @@ csched2_vcpu_migrate( > if ( unlikely(!cpumask_test_cpu(new_cpu, > cpupool_domain_cpumask(d))) ) > { > ASSERT(system_state == SYS_STATE_suspend); > + > if ( vcpu_on_runq(svc) ) > { > Stray new-line again. > @@ -3206,17 +3227,18 @@ csched2_runtime(const struct scheduler *ops, > int cpu, > * 2) If there's someone waiting whose credit is positive, > *run until your credit ~= his. > */ > -if ( ! list_empty(runq) ) > +if ( ! RB_EMPTY_ROOT(runq) ) > { > -struct csched2_vcpu *swait = runq_elem(runq->next); > +// Find the left most element, which is the most probable > candidate > +struct rb_node *node = rb_last(runq); > Comment style. And I think I'd say "Check the rightmost element in the tree, which is the one with the highest credits" > +struct csched2_vcpu *swait = runq_elem(node); > I think we can do: struct csched2_vcpu *swait = runq_elem(rb_last(runq)); Yeah, matter of taste, mostly. Still... Anyway, if you keep the code like this, no blanks in-between definitions. And you instead want one between the last definition and the if below. > if ( ! is_idle_vcpu(swait->vcpu) > && swait->credit > 0 ) > { > rt_credit = snext->credit - swait->credit; > } > } > - > /* > * The next guy on the runqueue may actually have a higher > credit, > Spurious change. > * if we've tried to avoid migrating him from a different cpu. > @@ -3350,9 +3372,8 @@ runq_candidate(struct csched2_runqueue_data > *rqd, > snext = csched2_vcpu(idle_vcpu[cpu]); > > check_runq: > -list_for_each_safe( iter, temp, >runq ) > -{ > -struct
Re: [Xen-devel] [PATCH v3 05/11] optee: add fast calls handling
On 17/01/2019 20:10, Volodymyr Babchuk wrote: Hi, Julien Grall writes: Hi, On 1/17/19 7:22 PM, Volodymyr Babchuk wrote: Julien Grall writes: [...] Because mediator can't share one static SHM region with all guests, it just disables it for all. I haven't seen an answer to my question on the previous version. For reminder: I'm sorry, I missed this somehow. Would it make sense to still allow the hardware domain to access static SHM? Not really. If linux drivers sees that OP-TEE support dynamic SHM, it uses it by default. And all new versions of OP-TEE support dynamic SHM. So, it will add unnecessary code to enable feature which will not be used. But, actually, there is a caveat. Right now linux driver requires static SHM, even if it will not use it. I have submitted patch that fixes it, but it still not merged. So... what do you think? Should I add piece of code which is needed right now, but will not be used later? I think it would be nice to have OP-TEE working in Dom0 with current Linux. For the guest, we can require a more recent Linux if there are no other solution. Okay, I'll take a look what can be done. Can I do this in a separate patch? I am fine with that. Please mention it in the commit message though. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL
Hi Jan, On 18/01/2019 11:09, Jan Beulich wrote: On 18.01.19 at 11:48, wrote: On 18/01/2019 09:54, Jan Beulich wrote: On 18.01.19 at 02:24, wrote: On Thu, 17 Jan 2019, Jan Beulich wrote: On 17.01.19 at 01:37, wrote: On Wed, 16 Jan 2019, Jan Beulich wrote: Stop. No. We very much can prove they are - _end points at one past the last element of _start[]. It is the compiler which can't prove the opposite, and hence it can't leverage undefined behavior for optimization purposes. You keep saying the compiler can't leverage it for optimization purpose, however there are confirmations that GCC may actually leverage it (e.g [1]). You actually need to trick the compiler to avoid the optimization (e.g RELOC_HIDE). Correct - that's the case I'm referring to when saying it can't leverage undefined behavior optimizations anymore. Without the hiding of course it can. But this trick is GCC specific, right? So we would need to have one trick for each compiler we support. Note that the solution originally suggested by Stefano has the same issue (i.e return unsigned long). Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen-unstable PVHdom0: Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at iommu.c:324
On 18/01/2019 13:50, Roger Pau Monné wrote: > On Fri, Jan 18, 2019 at 01:03:04PM +0100, Sander Eikelenboom wrote: >> Hi Roger, >> >> I gave PVH dom0 a spin, see how far I would get. > > Thanks! > >> With current xen-unstable unfortunately not that far, i got the splat below. > > Yes, this was already reported: > > https://lists.xenproject.org/archives/html/xen-devel/2019-01/msg01030.html >> If you need more info, would like me to test a patch (or some other git >> tree/branch), >> I will be happy to give it a spin ! > > Paul is working on a fix, but in the meantime just removing the > assertions should be fine: > > ---8<--- > diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c > index bd1af35a13..98e6fc35e2 100644 > --- a/xen/drivers/passthrough/iommu.c > +++ b/xen/drivers/passthrough/iommu.c > @@ -321,9 +321,6 @@ int iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn, > if ( !iommu_enabled || !hd->platform_ops ) > return 0; > > -ASSERT(IS_ALIGNED(dfn_x(dfn), (1ul << page_order))); > -ASSERT(IS_ALIGNED(mfn_x(mfn), (1ul << page_order))); > - > for ( i = 0; i < (1ul << page_order); i++ ) > { > rc = hd->platform_ops->map_page(d, dfn_add(dfn, i), mfn_add(mfn, i), > I gave that a spin and i now get a seemingly endless stream of IO_PAGE_FAULTs __ ___ __ ___ \ \/ /___ _ __ | || | / |___ \ / _ \_ __ ___ \ // _ \ '_ \ | || |_ | | __) || | | |__| '__/ __| / \ __/ | | | |__ _|| |/ __/ | |_| |__| | | (__ /_/\_\___|_| |_||_|(_)_|_(_)___/ |_| \___| (XEN) [001a375bd3d2] Xen version 4.12.0-rc (r...@dyndns.org) (gcc (Debian 6.3.0-18+deb9u1) 6.3.0 20170516) debug=y Fri Jan 18 14:47:30 CET 2019 (XEN) [001a3ecc2ebe] Latest ChangeSet: Mon Jan 14 14:59:37 2019 + git:50923ade7a-dirty (XEN) [001a438ee0b2] Bootloader: GRUB 2.02~beta3-5+deb9u1 (XEN) [001a46aeb5fb] Command line: dom0_mem=2048M,max:2048M loglvl=all console_timestamps=datems vga=gfx-1280x1024x32 no-cpuidle com1=38400,8n1 console=vga,com1 ivrs_ioapic[6]=00:14.0 iommu=on,verbose,debug conring_size=128k ucode=scan sched=credit2 gnttab_max_frames=64 dom0=pvh (XEN) [001a54d0d32a] Xen image load base address: 0 (XEN) [001a57a46f8b] Video information: (XEN) [001a59dfcfe2] VGA is graphics mode 1280x1024, 32 bpp (XEN) [001a5d25b312] VBE/DDC methods: V2; EDID transfer time: 1 seconds (XEN) [001a6103dee2] Disc information: (XEN) [001a633271db] Found 4 MBR signatures (XEN) [001a65ad4242] Found 4 EDD information structures (XEN) [001a68c0622e] Xen-e820 RAM map: (XEN) [001a6aeef328] - 00096400 (usable) (XEN) [001a6e80ff92] 00096400 - 000a (reserved) (XEN) [001a722c8335] 000e4000 - 0010 (reserved) (XEN) [001a75d7e455] 0010 - c7f9 (usable) (XEN) [001a796a0d3b] c7f9 - c7f9e000 (ACPI data) (XEN) [001a7d22145b] c7f9e000 - c7fe (ACPI NVS) (XEN) [001a80cd91ce] c7fe - c800 (reserved) (XEN) [001a84791428] ffe0 - 0001 (reserved) (XEN) [001a88248f32] 0001 - 00053800 (usable) (XEN) [001a91301890] New Xen image base address: 0xc780 (XEN) [001a946946fe] ACPI: RSDP 000FB100, 0014 (r0 ACPIAM) (XEN) [001a9795cb9d] ACPI: RSDT C7F9, 0048 (r1 MSIOEMSLIC 20100913 MSFT 97) (XEN) [001a9c58a075] ACPI: FACP C7F90200, 0084 (r1 7640MS A7640100 20100913 MSFT 97) (XEN) [001aa11b5236] ACPI: DSDT C7F905E0, 9427 (r1 A7640 A7640100 100 INTL 20051117) (XEN) [001aa5de0cca] ACPI: FACS C7F9E000, 0040 (XEN) [001aa8724396] ACPI: APIC C7F90390, 0088 (r1 7640MS A7640100 20100913 MSFT 97) (XEN) [001aad34ed70] ACPI: MCFG C7F90420, 003C (r1 7640MS OEMMCFG 20100913 MSFT 97) (XEN) [001ab1f7b106] ACPI: SLIC C7F90460, 0176 (r1 MSIOEMSLIC 20100913 MSFT 97) (XEN) [001ab6ba8663] ACPI: OEMB C7F9E040, 0072 (r1 7640MS A7640100 20100913 MSFT 97) (XEN) [001abb7d4723] ACPI: SRAT C7F9A5E0, 0108 (r3 AMDFAM_F_102 AMD 1) (XEN) [001ac0400a2a] ACPI: HPET C7F9A6F0, 0038 (r1 7640MS OEMHPET 20100913 MSFT 97) (XEN) [001ac502c9f6] ACPI: IVRS C7F9A730, 0108 (r1 AMD RD890S 202031 AMD 0) (XEN) [001ac9c581c5] ACPI: SSDT C7F9A840, 0DA4 (r1 A M I POWERNOW1 AMD 1) (XEN) [001ace883d6e] System RAM: 20479MB (20970648kB) (XEN) [001ad8bcf4cb] SRAT: PXM 0 -> APIC 00 -> Node 0 (XEN) [001adbaa0505] SRAT: PXM 0 -> APIC 01 -> Node 0 (XEN) [001ade96f9ea] SRAT: PXM 0 -> APIC 02 -> Node 0 (XEN) [001ae183f91b] SRAT: PXM 0 -> APIC 03 -> Node 0 (XEN) [001ae4710d2a] SRAT: PXM 0 -> APIC 04 -> Node 0 (XEN) [001ae75e13aa] SRAT: PXM 0 -> APIC 05 -> Node 0 (XEN)
Re: [Xen-devel] [PATCH v3 3/3] xen: add CONFIG item for default dom0 memory size
On 18/01/2019 14:44, Andrew Cooper wrote: > On 18/01/2019 13:19, Juergen Gross wrote: >> On 18/01/2019 14:13, Andrew Cooper wrote: >>> On 10/12/2018 11:44, Juergen Gross wrote: With being able to specify a dom0_mem value depending on host memory size on x86 make it easy for distros to specify a default dom0 size by adding a CONFIG_DOM0_MEM item which presets the dom0_mem boot parameter value. It will be used only if no dom0_mem parameter was specified in the boot parameters. Signed-off-by: Juergen Gross Reviewed-by: Jan Beulich >>> Why was this patch accepted? We've already got a suitable Kconfig >>> option for this, CONFIG_CMDLINE. >> Why do we have a config option for the default scheduler? > > That pre-dates CONFIG_CMDLINE, but is of dubious use. I don't agree with you here. I think this is a very sensible choice. > >> >> And something like: >> >> dom0_mem=max:8G dom0_mem=10% >> >> is different from >> >> CONFIG_DOM0_MEM="max:8G" plus dom0_mem=10% >> >> on a host with say 100G of memory: In the first case this would be 8G, >> in the second case 10G. > > And? There is no way of preventing that. I wanted to say: using CONFIG_DOM0_MEM and CONFIG_CMDLINE are different. The former is evaluated only if the user doesn't specify dom0_mem as a parameter, while the latter _is_ cumulative. The advantage of CONFIG_DOM0_MEM is that is really only the default, not an additional setting the user has to take into acount. > A user can really put two dom0_mem= on the real command line, and could > put a dom0_mem in CONFIG_CMDLINE. Right, and then he'd have three cumulative settings. > The only solution to this problem is for the final dom0_mem= to be a > full specification, and I still see no use for CONFIG_DOM0_MEM So a user specifying dom0_mem in a 4.11 setup would eventually need to modify his setting. With CONFIG_DOM0_MEM he doesn't have to do that. Additionally CONFIG_CMDLINE is available in expert mode only, while CONFIG_DOM0_MEM can be set more easily. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] libxl: fix error message for unsharing namespaces
On 18/01/2019 15:06, Ian Jackson wrote: > Wei Liu writes ("[PATCH] libxl: fix error message for unsharing namespaces"): >> Signed-off-by: Wei Liu > > Thanks. IMO this simple bugfix should go into 4.12. I agree (while I kind of like the "unfailed" :-) ). Release-acked-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] libxl: fix error message for unsharing namespaces
Wei Liu writes ("[PATCH] libxl: fix error message for unsharing namespaces"): > Signed-off-by: Wei Liu Thanks. IMO this simple bugfix should go into 4.12. Acked-by: Ian Jackson Ian. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/3] xen: add CONFIG item for default dom0 memory size
On 18/01/2019 13:19, Juergen Gross wrote: > On 18/01/2019 14:13, Andrew Cooper wrote: >> On 10/12/2018 11:44, Juergen Gross wrote: >>> With being able to specify a dom0_mem value depending on host memory >>> size on x86 make it easy for distros to specify a default dom0 size by >>> adding a CONFIG_DOM0_MEM item which presets the dom0_mem boot parameter >>> value. >>> >>> It will be used only if no dom0_mem parameter was specified in the >>> boot parameters. >>> >>> Signed-off-by: Juergen Gross >>> Reviewed-by: Jan Beulich >> Why was this patch accepted? We've already got a suitable Kconfig >> option for this, CONFIG_CMDLINE. > Why do we have a config option for the default scheduler? That pre-dates CONFIG_CMDLINE, but is of dubious use. > > And something like: > > dom0_mem=max:8G dom0_mem=10% > > is different from > > CONFIG_DOM0_MEM="max:8G" plus dom0_mem=10% > > on a host with say 100G of memory: In the first case this would be 8G, > in the second case 10G. And? There is no way of preventing that. A user can really put two dom0_mem= on the real command line, and could put a dom0_mem in CONFIG_CMDLINE. The only solution to this problem is for the final dom0_mem= to be a full specification, and I still see no use for CONFIG_DOM0_MEM ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/3] xen: add CONFIG item for default dom0 memory size
On 18/01/2019 14:13, Andrew Cooper wrote: > On 10/12/2018 11:44, Juergen Gross wrote: >> With being able to specify a dom0_mem value depending on host memory >> size on x86 make it easy for distros to specify a default dom0 size by >> adding a CONFIG_DOM0_MEM item which presets the dom0_mem boot parameter >> value. >> >> It will be used only if no dom0_mem parameter was specified in the >> boot parameters. >> >> Signed-off-by: Juergen Gross >> Reviewed-by: Jan Beulich > > Why was this patch accepted? We've already got a suitable Kconfig > option for this, CONFIG_CMDLINE. Why do we have a config option for the default scheduler? And something like: dom0_mem=max:8G dom0_mem=10% is different from CONFIG_DOM0_MEM="max:8G" plus dom0_mem=10% on a host with say 100G of memory: In the first case this would be 8G, in the second case 10G. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/3] xen: add CONFIG item for default dom0 memory size
On 10/12/2018 11:44, Juergen Gross wrote: > With being able to specify a dom0_mem value depending on host memory > size on x86 make it easy for distros to specify a default dom0 size by > adding a CONFIG_DOM0_MEM item which presets the dom0_mem boot parameter > value. > > It will be used only if no dom0_mem parameter was specified in the > boot parameters. > > Signed-off-by: Juergen Gross > Reviewed-by: Jan Beulich Why was this patch accepted? We've already got a suitable Kconfig option for this, CONFIG_CMDLINE. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen-unstable PVHdom0: Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at iommu.c:324
On Fri, Jan 18, 2019 at 01:03:04PM +0100, Sander Eikelenboom wrote: > Hi Roger, > > I gave PVH dom0 a spin, see how far I would get. Thanks! > With current xen-unstable unfortunately not that far, i got the splat below. Yes, this was already reported: https://lists.xenproject.org/archives/html/xen-devel/2019-01/msg01030.html > If you need more info, would like me to test a patch (or some other git > tree/branch), > I will be happy to give it a spin ! Paul is working on a fix, but in the meantime just removing the assertions should be fine: ---8<--- diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c index bd1af35a13..98e6fc35e2 100644 --- a/xen/drivers/passthrough/iommu.c +++ b/xen/drivers/passthrough/iommu.c @@ -321,9 +321,6 @@ int iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn, if ( !iommu_enabled || !hd->platform_ops ) return 0; -ASSERT(IS_ALIGNED(dfn_x(dfn), (1ul << page_order))); -ASSERT(IS_ALIGNED(mfn_x(mfn), (1ul << page_order))); - for ( i = 0; i < (1ul << page_order); i++ ) { rc = hd->platform_ops->map_page(d, dfn_add(dfn, i), mfn_add(mfn, i), ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] libxl: fix error message for unsharing namespaces
Signed-off-by: Wei Liu --- tools/libxl/libxl_linux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c index 59dd945bc1..873b0271af 100644 --- a/tools/libxl/libxl_linux.c +++ b/tools/libxl/libxl_linux.c @@ -344,7 +344,7 @@ int libxl__local_dm_preexec_restrict(libxl__gc *gc) /* Unshare mount and IPC namespaces. These are unused by QEMU. */ r = unshare(CLONE_NEWNS | CLONE_NEWIPC); if (r) { -LOGE(ERROR, "libxl: Mount and IPC namespace unfailed"); +LOGE(ERROR, "libxl: unshare Mount and IPC namespace failed"); return ERROR_FAIL; } -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-next v2] xen: make grant table configurable
Introduce CONFIG_GRANT_TABLE. Provide stubs and make sure x86 and arm hypervisors build with grant table disabled. Signed-off-by: Wei Liu --- v2: 1. CONFIG_GRANT_TABLE should depend on EXPERT=y 2. Define two opt_* to be 0 --- xen/arch/arm/traps.c | 2 ++ xen/arch/x86/hvm/Makefile | 2 +- xen/arch/x86/hvm/hypercall.c | 4 xen/arch/x86/hypercall.c | 2 ++ xen/arch/x86/pv/Makefile | 2 +- xen/arch/x86/pv/hypercall.c | 2 ++ xen/common/Kconfig| 11 ++ xen/common/Makefile | 2 +- xen/include/xen/grant_table.h | 48 +++ 9 files changed, 72 insertions(+), 3 deletions(-) diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 221c762ada..0f1c1b6431 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -1392,7 +1392,9 @@ static arm_hypercall_t arm_hypercall_table[] = { HYPERCALL_DEPRECATED(physdev_op_compat, 1), HYPERCALL(sysctl, 2), HYPERCALL(hvm_op, 2), +#ifdef CONFIG_GRANT_TABLE HYPERCALL(grant_table_op, 3), +#endif HYPERCALL(multicall, 2), HYPERCALL(platform_op, 1), HYPERCALL_ARM(vcpu_op, 3), diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile index 86b106f8e7..43e5f3a21f 100644 --- a/xen/arch/x86/hvm/Makefile +++ b/xen/arch/x86/hvm/Makefile @@ -7,7 +7,7 @@ obj-y += dm.o obj-bin-y += dom0_build.init.o obj-y += domain.o obj-y += emulate.o -obj-y += grant_table.o +obj-$(CONFIG_GRANT_TABLE) += grant_table.o obj-y += hpet.o obj-y += hvm.o obj-y += hypercall.o diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c index 19d126377a..1f667efc36 100644 --- a/xen/arch/x86/hvm/hypercall.c +++ b/xen/arch/x86/hvm/hypercall.c @@ -47,6 +47,7 @@ static long hvm_memory_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) return rc; } +#ifdef CONFIG_GRANT_TABLE static long hvm_grant_table_op( unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) uop, unsigned int count) { @@ -71,6 +72,7 @@ static long hvm_grant_table_op( else return compat_grant_table_op(cmd, uop, count); } +#endif static long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) { @@ -119,7 +121,9 @@ static long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) static const hypercall_table_t hvm_hypercall_table[] = { HVM_CALL(memory_op), +#ifdef CONFIG_GRANT_TABLE HVM_CALL(grant_table_op), +#endif COMPAT_CALL(vcpu_op), HVM_CALL(physdev_op), COMPAT_CALL(xen_version), diff --git a/xen/arch/x86/hypercall.c b/xen/arch/x86/hypercall.c index 032de8f8f8..9311b63c1e 100644 --- a/xen/arch/x86/hypercall.c +++ b/xen/arch/x86/hypercall.c @@ -47,7 +47,9 @@ const hypercall_args_t hypercall_args_table[NR_hypercalls] = ARGS(xen_version, 2), ARGS(console_io, 3), ARGS(physdev_op_compat, 1), +#ifdef CONFIG_GRANT_TABLE ARGS(grant_table_op, 3), +#endif ARGS(vm_assist, 2), COMP(update_va_mapping_otherdomain, 4, 5), ARGS(vcpu_op, 3), diff --git a/xen/arch/x86/pv/Makefile b/xen/arch/x86/pv/Makefile index 65bca04175..cf28434ba9 100644 --- a/xen/arch/x86/pv/Makefile +++ b/xen/arch/x86/pv/Makefile @@ -5,7 +5,7 @@ obj-y += emulate.o obj-y += emul-gate-op.o obj-y += emul-inv-op.o obj-y += emul-priv-op.o -obj-y += grant_table.o +obj-$(CONFIG_GRANT_TABLE) += grant_table.o obj-y += hypercall.o obj-y += iret.o obj-y += misc-hypercalls.o diff --git a/xen/arch/x86/pv/hypercall.c b/xen/arch/x86/pv/hypercall.c index 5d11911735..ee0a6da515 100644 --- a/xen/arch/x86/pv/hypercall.c +++ b/xen/arch/x86/pv/hypercall.c @@ -53,7 +53,9 @@ const hypercall_table_t pv_hypercall_table[] = { COMPAT_CALL(xen_version), HYPERCALL(console_io), COMPAT_CALL(physdev_op_compat), +#ifdef CONFIG_GRANT_TABLE COMPAT_CALL(grant_table_op), +#endif COMPAT_CALL(vm_assist), COMPAT_CALL(update_va_mapping_otherdomain), COMPAT_CALL(iret), diff --git a/xen/common/Kconfig b/xen/common/Kconfig index a79cd40441..24a1c75ec1 100644 --- a/xen/common/Kconfig +++ b/xen/common/Kconfig @@ -11,6 +11,17 @@ config COMPAT config CORE_PARKING bool +config GRANT_TABLE + bool "Grant table support" if EXPERT = "y" + default y + ---help--- + Grant table provides a generic mechanism to memory sharing + between domains. This shared memory interface underpins the + split device drivers for block and network IO in a classic + Xen setup. + + If unsure, say Y. + config HAS_ALTERNATIVE bool diff --git a/xen/common/Makefile b/xen/common/Makefile index 56fc201b6b..e748554a44 100644 --- a/xen/common/Makefile +++ b/xen/common/Makefile @@ -10,7 +10,7 @@ obj-y += event_2l.o obj-y += event_channel.o obj-y += event_fifo.o obj-$(CONFIG_CRASH_DEBUG) += gdbstub.o -obj-y += grant_table.o +obj-$(CONFIG_GRANT_TABLE) += grant_table.o obj-y += guestcopy.o obj-bin-y += gunzip.init.o obj-y += irq.o diff --git a/xen/include/xen/grant_table.h
Re: [Xen-devel] [PATCH 0/2] add xl command to get hypervisor .config
On 18/01/2019 12:38, George Dunlap wrote: > On 1/17/19 3:23 PM, Juergen Gross wrote: >> On 17/01/2019 15:57, Juergen Gross wrote: >>> Add "xl get-config" printing the .config used to build the currently >>> running hypervisor. >> >> BTW: I'd like to have feedback especially if someone thinks this series >> should by any means be part of 4.12. If not I'll send out V2 with >> bumping the sysctl interface version. > > If someone *else* would have send this instead of you, would you have > considered including it, or would you have said that this is way past > the "feature freeze" in mid-December, it will be just fine to wait until > 4.13? It would need a rather strong reason to be accepted. > If the answer is "yes", then it begs the question what the point of the > feature freeze is, if new features can be introduced after the *code > freeze*. > > If the answer is "no", then you're basically abusing your position as > release manager to get your own code in. That's exactly why I asked for that feedback. Someone saying "I don't mind this going in" is not enough. I said: "... should by any means be part of 4.12". So there would need to be a strong reason for that request, and up to now I haven't seen that reason. > *Everyone* thinks that their own patch is super-important to get in > before the next release, and basically perfect and not going to cause > any regressions. It's the job of a release manager to be the "bad guy" > and tell them no; and a release manager should be stricter with > themselves than with anybody else. I completely agree with you here. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Xen-unstable PVHdom0: Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at iommu.c:324
Hi Roger, I gave PVH dom0 a spin, see how far I would get. With current xen-unstable unfortunately not that far, i got the splat below. If you need more info, would like me to test a patch (or some other git tree/branch), I will be happy to give it a spin ! -- Sander __ ___ __ ___ \ \/ /___ _ __ | || | / |___ \ / _ \_ __ ___ \ // _ \ '_ \ | || |_ | | __) || | | |__| '__/ __| / \ __/ | | | |__ _|| |/ __/ | |_| |__| | | (__ /_/\_\___|_| |_||_|(_)_|_(_)___/ |_| \___| (XEN) [001a2edd3456] Xen version 4.12.0-rc (r...@dyndns.org) (gcc (Debian 6.3.0-18+deb9u1) 6.3.0 20170516) debug=y Fri Jan 18 12:40:33 CET 2019 (XEN) [001a364d8a16] Latest ChangeSet: Mon Jan 14 14:59:37 2019 + git:50923ade7a-dirty (XEN) [001a3b103720] Bootloader: GRUB 2.02~beta3-5+deb9u1 (XEN) [001a3e301032] Command line: dom0_mem=2048M,max:2048M loglvl=all console_timestamps=datems vga=gfx-1280x1024x32 no-cpuidle com1=38400,8n1 console=vga,com1 ivrs_ioapic[6]=00:14.0 iommu=on,verbose,debug conring_size=128k ucode=scan sched=credit2 gnttab_max_frames=64 dom0=pvh (XEN) [001a4c523f88] Xen image load base address: 0 (XEN) [001a4f25e0ce] Video information: (XEN) [001a516122f2] VGA is graphics mode 1280x1024, 32 bpp (XEN) [001a54a70e70] VBE/DDC methods: V2; EDID transfer time: 1 seconds (XEN) [001a58853aa0] Disc information: (XEN) [001a5ab3d4b3] Found 4 MBR signatures (XEN) [001a5d2ea75b] Found 4 EDD information structures (XEN) [001a6041c91e] Xen-e820 RAM map: (XEN) [001a627060dd] - 00096400 (usable) (XEN) [001a660275e3] 00096400 - 000a (reserved) (XEN) [001a69add13e] 000e4000 - 0010 (reserved) (XEN) [001a6d594f0e] 0010 - c7f9 (usable) (XEN) [001a70eb6416] c7f9 - c7f9e000 (ACPI data) (XEN) [001a74a37cce] c7f9e000 - c7fe (ACPI NVS) (XEN) [001a784ef83d] c7fe - c800 (reserved) (XEN) [001a7bfa7c9d] ffe0 - 0001 (reserved) (XEN) [001a7fa5daee] 0001 - 00053800 (usable) (XEN) [001a88afa99a] New Xen image base address: 0xc780 (XEN) [001a8be8c9cd] ACPI: RSDP 000FB100, 0014 (r0 ACPIAM) (XEN) [001a8f15516d] ACPI: RSDT C7F9, 0048 (r1 MSIOEMSLIC 20100913 MSFT 97) (XEN) [001a93d81daa] ACPI: FACP C7F90200, 0084 (r1 7640MS A7640100 20100913 MSFT 97) (XEN) [001a989ad923] ACPI: DSDT C7F905E0, 9427 (r1 A7640 A7640100 100 INTL 20051117) (XEN) [001a9d5d875a] ACPI: FACS C7F9E000, 0040 (XEN) [001a9ff1c7b3] ACPI: APIC C7F90390, 0088 (r1 7640MS A7640100 20100913 MSFT 97) (XEN) [001aa4b484e6] ACPI: MCFG C7F90420, 003C (r1 7640MS OEMMCFG 20100913 MSFT 97) (XEN) [001aa9773f16] ACPI: SLIC C7F90460, 0176 (r1 MSIOEMSLIC 20100913 MSFT 97) (XEN) [001aae39ed46] ACPI: OEMB C7F9E040, 0072 (r1 7640MS A7640100 20100913 MSFT 97) (XEN) [001ab2fcbdf3] ACPI: SRAT C7F9A5E0, 0108 (r3 AMDFAM_F_102 AMD 1) (XEN) [001ab7bf6e3a] ACPI: HPET C7F9A6F0, 0038 (r1 7640MS OEMHPET 20100913 MSFT 97) (XEN) [001abc8243f6] ACPI: IVRS C7F9A730, 0108 (r1 AMD RD890S 202031 AMD 0) (XEN) [001ac144f4d5] ACPI: SSDT C7F9A840, 0DA4 (r1 A M I POWERNOW1 AMD 1) (XEN) [001ac607b9b8] System RAM: 20479MB (20970648kB) (XEN) [001ad03a902b] SRAT: PXM 0 -> APIC 00 -> Node 0 (XEN) [001ad3277e5e] SRAT: PXM 0 -> APIC 01 -> Node 0 (XEN) [001ad6148a58] SRAT: PXM 0 -> APIC 02 -> Node 0 (XEN) [001ad9019acb] SRAT: PXM 0 -> APIC 03 -> Node 0 (XEN) [001adbeea038] SRAT: PXM 0 -> APIC 04 -> Node 0 (XEN) [001adedbaa18] SRAT: PXM 0 -> APIC 05 -> Node 0 (XEN) [001ae1c8a3ab] SRAT: Node 0 PXM 0 0-a (XEN) [001ae4698e55] SRAT: Node 0 PXM 0 10-c800 (XEN) [001ae76fe63a] SRAT: Node 0 PXM 0 1-53800 (XEN) [001aeaa91502] NUMA: Allocated memnodemap from 5334dc000 - 5334e2000 (XEN) [001aeea0cfeb] NUMA: Using 8 for the hash shift. (XEN) [001b34b66796] Domain heap initialised (XEN) [001b37313a56] Allocated console ring of 128 KiB. (XEN) [001b4d5599fd] vesafb: framebuffer at 0xd000, mapped to 0x82c000201000, using 6144k, total 16384k (XEN) [001b5322fbe3] vesafb: mode is 1280x1024x32, linelength=5120, font 8x16 (XEN) [001b5740c1a6] vesafb: Truecolor: size=0:8:8:8, shift=0:16:8:0 (XEN) [001b5aecec50] CPU Vendor: AMD, Family 16 (0x10), Model 10 (0xa), Stepping 0 (raw 00100fa0) (XEN) [001b64e9ba53] found SMP MP-table at 000ff780 (XEN) [001b67bd596d] DMI present. (XEN) [001b69ac60a3] Using APIC driver default (XEN) [001b6c408c8b] ACPI: PM-Timer IO Port: 0x808 (24 bits) (XEN) [001b6f8664fd] ACPI: SLEEP INFO:
Re: [Xen-devel] [PATCH v2] drm/xen-front: Make shmem backed display buffer coherent
(+ Stefano) Hi, Sorry for jumping late in the conversation. On 18/01/2019 09:40, Oleksandr Andrushchenko wrote: On 1/17/19 11:18 AM, Christoph Hellwig wrote: On Wed, Jan 16, 2019 at 06:43:29AM +, Oleksandr Andrushchenko wrote: This whole issue keeps getting more and more confusing. Well, I don't really do DMA here, but instead the buffers in question are shared with other Xen domain, so effectively it could be thought of some sort of DMA here, where the "device" is that remote domain. If the buffers are not flushed then the remote part sees some inconsistency which in my case results in artifacts on screen while displaying the buffers. When buffers are allocated via DMA API then there are no artifacts; if buffers are allocated with shmem + DMA mapping then there are no artifacts as well. The only offending use-case is when I use shmem backed buffers, but do not flush them The right answer would be to implement cache maintainance hooks for this case in the Xen arch code. These would basically look the same as the low-level cache maintainance used by the DMA ops, but without going through the DMA mapping layer, in fact they should probably reuse the same low-level assembly routines. I don't think this is the first usage of such Xen buffer sharing, so what do the other users do? I'll have to get even deeper into it. Initially I looked at the code, but didn't find anything useful. Or maybe I have just overlooked obvious things there From Xen on Arm ABI: "All memory which is shared with other entities in the system (including the hypervisor and other guests) must reside in memory which is mapped as Normal Inner Write-Back Outer Write-Back Inner-Shareable. This applies to: - hypercall arguments passed via a pointer to guest memory. - memory shared via the grant table mechanism (including PV I/O rings etc). - memory shared with the hypervisor (struct shared_info, struct vcpu_info, the grant table, etc). " So you should not need any cache maintenance here. Can you provide more details on the memory attribute you use for memory shared in both the backend and frontend? Cheers, Thank you, Oleksandr ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/2] add xl command to get hypervisor .config
On 1/17/19 3:23 PM, Juergen Gross wrote: > On 17/01/2019 15:57, Juergen Gross wrote: >> Add "xl get-config" printing the .config used to build the currently >> running hypervisor. > > BTW: I'd like to have feedback especially if someone thinks this series > should by any means be part of 4.12. If not I'll send out V2 with > bumping the sysctl interface version. If someone *else* would have send this instead of you, would you have considered including it, or would you have said that this is way past the "feature freeze" in mid-December, it will be just fine to wait until 4.13? If the answer is "yes", then it begs the question what the point of the feature freeze is, if new features can be introduced after the *code freeze*. If the answer is "no", then you're basically abusing your position as release manager to get your own code in. *Everyone* thinks that their own patch is super-important to get in before the next release, and basically perfect and not going to cause any regressions. It's the job of a release manager to be the "bad guy" and tell them no; and a release manager should be stricter with themselves than with anybody else. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [qemu-mainline bisection] complete build-i386
> -Original Message- > From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf > Of osstest service owner > Sent: 18 January 2019 09:40 > To: xen-devel@lists.xenproject.org; osstest-ad...@xenproject.org > Subject: [Xen-devel] [qemu-mainline bisection] complete build-i386 > > branch xen-unstable > xenbranch xen-unstable > job build-i386 > testid xen-build > > Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git > Tree: qemuu git://git.qemu.org/qemu.git > Tree: xen git://xenbits.xen.org/xen.git > > *** Found and reproduced problem changeset *** > > Bug is in tree: qemuu git://git.qemu.org/qemu.git > Bug introduced: b6af8926fb858c4f1426e5acb2cfc1f0580ec98a > Bug not present: d4683cf952d3bdcbcbfcfd982d77cb6b02041040 > Last fail repro: http://logs.test- > lab.xenproject.org/osstest/logs/132052/ > > > commit b6af8926fb858c4f1426e5acb2cfc1f0580ec98a > Author: Paul Durrant > Date: Tue Jan 8 14:48:59 2019 + > > xen: add implementations of xen-block connect and disconnect > functions... > > ...and wire in the dataplane. > > This patch adds the remaining code to make the xen-block XenDevice > functional. The parameters that a block frontend expects to find are > populated in the backend xenstore area, and the 'ring-ref' and > 'event-channel' values specified in the frontend xenstore area are > mapped/bound and used to set up the dataplane. > > Signed-off-by: Paul Durrant > Reviewed-by: Anthony Perard > Signed-off-by: Anthony PERARD > FTR, the problem is: /home/osstest/build.132052.build-i386/xen/tools/qemu-xen-dir/hw/block/xen-block.c: In function 'xen_block_realize': /home/osstest/build.132052.build-i386/xen/tools/qemu-xen-dir/hw/block/xen-block.c:212:31: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'int64_t' [-Werror=format=] conf->logical_block_size); ^ cc1: all warnings being treated as errors /home/osstest/build.132052.build-i386/xen/tools/qemu-xen-dir/rules.mak:69: recipe for target 'hw/block/xen-block.o' failed A fix for this has been posted to qemu-devel: https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg03721.html Paul > > For bisection revision-tuple graph see: >http://logs.test-lab.xenproject.org/osstest/results/bisect/qemu- > mainline/build-i386.xen-build.html > Revision IDs in each graph node refer, respectively, to the Trees above. > > > Running cs-bisection-step --graph-out=/home/logs/results/bisect/qemu- > mainline/build-i386.xen-build --summary-out=tmp/132052.bisection-summary - > -basis-template=131842 --blessings=real,real-bisect qemu-mainline build- > i386 xen-build > Searching for failure / basis pass: > 131980 fail [host=debina1] / 131842 [host=baroque1] 131801 ok. > Failure / basis pass flights: 131980 / 131801 > (tree with no url: minios) > (tree with no url: ovmf) > (tree with no url: seabios) > Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git > Tree: qemuu git://git.qemu.org/qemu.git > Tree: xen git://xenbits.xen.org/xen.git > Latest d0d8ad39ecb51cd7497cd524484fe09f50876798 > 4b9f0b0f7c84eea2dfb0d5be3e0254bc91319dbc > 93a62c544e20ba9e141e411bbaae3d65259d13a3 > Basis pass d0d8ad39ecb51cd7497cd524484fe09f50876798 > c102d9471f8f02d9fbea72ec4505d7089173f470 > a5b0eb363694e7e15405f0b3fc5fb6fab79df1db > Generating revisions with ./adhoc-revtuple-generator > git://xenbits.xen.org/qemu-xen- > traditional.git#d0d8ad39ecb51cd7497cd524484fe09f50876798- > d0d8ad39ecb51cd7497cd524484fe09f50876798 > git://git.qemu.org/qemu.git#c102d9471f8f02d9fbea72ec4505d7089173f470- > 4b9f0b0f7c84eea2dfb0d5be3e0254bc91319dbc > git://xenbits.xen.org/xen.git#a5b0eb363694e7e15405f0b3fc5fb6fab79df1db- > 93a62c544e20ba9e141e411bbaae3d65259d13a3 > Loaded 5051 nodes in revision graph > Searching for test results: > 131801 pass d0d8ad39ecb51cd7497cd524484fe09f50876798 > c102d9471f8f02d9fbea72ec4505d7089173f470 > a5b0eb363694e7e15405f0b3fc5fb6fab79df1db > 131842 [host=baroque1] > 131980 fail d0d8ad39ecb51cd7497cd524484fe09f50876798 > 4b9f0b0f7c84eea2dfb0d5be3e0254bc91319dbc > 93a62c544e20ba9e141e411bbaae3d65259d13a3 > 131963 [host=debina0] > 132047 pass d0d8ad39ecb51cd7497cd524484fe09f50876798 > d4683cf952d3bdcbcbfcfd982d77cb6b02041040 > 93a62c544e20ba9e141e411bbaae3d65259d13a3 > 132028 [host=debina0] > 132029 [host=debina0] > 132048 fail d0d8ad39ecb51cd7497cd524484fe09f50876798 > b6af8926fb858c4f1426e5acb2cfc1f0580ec98a > 93a62c544e20ba9e141e411bbaae3d65259d13a3 > 132030 [host=debina0] > 132031 pass d0d8ad39ecb51cd7497cd524484fe09f50876798 > c102d9471f8f02d9fbea72ec4505d7089173f470 > a5b0eb363694e7e15405f0b3fc5fb6fab79df1db > 132049 pass d0d8ad39ecb51cd7497cd524484fe09f50876798 > d4683cf952d3bdcbcbfcfd982d77cb6b02041040 > 93a62c544e20ba9e141e411bbaae3d65259d13a3 > 132033 fail
Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL
>>> On 18.01.19 at 11:48, wrote: > On 18/01/2019 09:54, Jan Beulich wrote: > On 18.01.19 at 02:24, wrote: >>> On Thu, 17 Jan 2019, Jan Beulich wrote: >>> On 17.01.19 at 01:37, wrote: > On Wed, 16 Jan 2019, Jan Beulich wrote: >> Stop. No. We very much can prove they are - _end points at >> one past the last element of _start[]. It is the compiler which >> can't prove the opposite, and hence it can't leverage >> undefined behavior for optimization purposes. > > You keep saying the compiler can't leverage it for optimization purpose, > however > there are confirmations that GCC may actually leverage it (e.g [1]). You > actually need to trick the compiler to avoid the optimization (e.g > RELOC_HIDE). Correct - that's the case I'm referring to when saying it can't leverage undefined behavior optimizations anymore. Without the hiding of course it can. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL
Hi Jan, On 18/01/2019 09:54, Jan Beulich wrote: On 18.01.19 at 02:24, wrote: On Thu, 17 Jan 2019, Jan Beulich wrote: On 17.01.19 at 01:37, wrote: On Wed, 16 Jan 2019, Jan Beulich wrote: Stop. No. We very much can prove they are - _end points at one past the last element of _start[]. It is the compiler which can't prove the opposite, and hence it can't leverage undefined behavior for optimization purposes. You keep saying the compiler can't leverage it for optimization purpose, however there are confirmations that GCC may actually leverage it (e.g [1]). You actually need to trick the compiler to avoid the optimization (e.g RELOC_HIDE). So obviously, this is not only a MISRA "problem" as you state here and below. I believe Stefano, Stewart and I provided plenty of documentation/thread to support our positions. Can you provide us documentation/thread showing the compiler will not try to leverage that case? Cheers, [1] https://kristerw.blogspot.com/2016/12/pointer-comparison-invalid-optimization.html?m=1 -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/2] add xl command to get hypervisor .config
On Thu, Jan 17, 2019 at 04:23:31PM +0100, Juergen Gross wrote: > On 17/01/2019 15:57, Juergen Gross wrote: > > Add "xl get-config" printing the .config used to build the currently > > running hypervisor. > > BTW: I'd like to have feedback especially if someone thinks this series > should by any means be part of 4.12. If not I'll send out V2 with > bumping the sysctl interface version. I certainly won't have objection for it to go into 4.12. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [GIT PULL] xen: fixes for 5.0-rc3
Linus, Please git pull the following tag: git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git for-linus-5.0-rc3-tag xen: fixes for 5.0-rc3 It contains: - Several fixes for the Xen pvcalls drivers (1 fix for the backend and 8 for the frontend). - A fix for a rather longstanding bug in the Xen sched_clock() interface which led to weird time jumps when migrating the system. - A fix for avoiding accesses to x2apic MSRs in Xen PV guests. Thanks. Juergen arch/x86/xen/enlighten_pv.c | 5 +- arch/x86/xen/time.c | 12 +++-- drivers/xen/events/events_base.c | 2 +- drivers/xen/pvcalls-back.c | 9 ++-- drivers/xen/pvcalls-front.c | 104 --- 5 files changed, 90 insertions(+), 42 deletions(-) Juergen Gross (1): xen: Fix x86 sched_clock() interface for xen Stefano Stabellini (5): pvcalls-front: read all data before closing the connection pvcalls-front: don't try to free unallocated rings pvcalls-front: properly allocate sk pvcalls-front: don't return error when the ring is full pvcalls-back: set -ENOTCONN in pvcalls_conn_back_read Talons Lee (1): always clear the X2APIC_ENABLE bit for PV guest Wen Yang (2): pvcalls-front: Avoid get_free_pages(GFP_KERNEL) under spinlock pvcalls-front: fix potential null dereference YueHaibing (1): xen/pvcalls: remove set but not used variable 'intf' ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL
>>> On 18.01.19 at 02:24, wrote: > On Thu, 17 Jan 2019, Jan Beulich wrote: >> >>> On 17.01.19 at 01:37, wrote: >> > On Wed, 16 Jan 2019, Jan Beulich wrote: >> >> In any event - since intermediate variables merely hide the >> >> casting from the compiler, but they don't remove the casts, the >> >> solution involving casts is better imo, for incurring less overhead. >> > >> > This is where I completely disagree. The intermediate variables are not >> > hiding casts from the compiler. There were never any pointers in this >> > case. The linker creates "symbols", not pointers, completely invisible >> > from C land. Assembly uses these symbols to initialize variables. We >> > expose these assembly variables as integer to C lands. LD scripts and >> > assembly have their own terminology and rules: neither "_start" nor >> > "start" are pointers at any point in time. The operations done in var.S >> > is not a cast. The C spec is happy, the compiler is happy, MISRA-C is >> > happy. And we get to avoid the ugly SYMBOL macro that Linux uses. It is >> > really a win-win. >> >> Well, that's a position one can take. But we have to settle on another >> aspect then first: Does what is not done in C underly C's rules? I >> thought you were of the opinion that what comes from linker scripts >> does. In which case what comes from assembly files ought to, too. >> (FAOD my implication is: If the answer is yes, both approaches >> violate C's rules. If the answer is no, no change is needed at all.) > > Great question, that is the core of the issue. Also, let me premise that > I agree on the comments you made on the patches (I dislike "start_" > too), and I can address them if we agree to continue down this path. > > But no, I do not think that what is done outside of C-land should follow > C rules. But I do not agree with your conclusion that in that case there > is no difference between the approaches. Let's get more into the > details. > > > 1) SYMBOL_HIDE returning pointer type > > Let's take _start and _end as an example. _start is born as a linker > symbol, and it becomes a C pointer when we do: > > extern char _start[], _end[] > > Now it is a pointer (actually I should say an array, but let's pretend > they are the same thing for this discussion). > > When we do: > > SYMBOL_HIDE(_end) - SYMBOL_HIDE(_start) > > We are still subtracting pointers: the pointers returned by SYMBOL_HIDE. > We cannot prove that they are pointers to the same object or subsequence > objects in memory, so it is undefined behavior, which is not allowed. Stop. No. We very much can prove they are - _end points at one past the last element of _start[]. It is the compiler which can't prove the opposite, and hence it can't leverage undefined behavior for optimization purposes. > 3) var.S + start_ as unsigned long > > With this approach, _start is born as a linker symbol. It is never > exported to C, so from C point of view, it doesn't exist. There is > another variable named "start_" defined in assembly and initialized to > _start. Now we go into C land with: > > extern uintptr_t start_, end_ > > start_ and end_ are uintptr_t from the beginning from C point of view. > They have never been pointers or in any way connected to _start. They > are "clean". > > When we do: > > _end - _start > > it is a subtraction between uintptr_t, which is allowed. When we do: > > for ( call = (const initcall_t *)initcall_start_; > (uintptr_t)call < presmp_initcall_end_; > > The comparison is still between uintptr_t types, and the value of "call" > still comes from an unsigned long initially. There is never a comparison > between dubious pointers. (Interger to pointer conversions and pointer > to integer conversions are allowed by MISRA with some limitations, but I > am double-checking.) Even: > >(uintptr_t)random_pointer < presmp_initcall_end_ > > would be acceptable because presmp_initcall_end_ is an integer and has > always been an integer from C point of view. Well, as said - this is one of the possible positions to take. Personally I see no difference between the helper symbols defined in assembly sources, or in C sources the object files for which are never made part of potential whole program optimization. Using C files for this is still in conflict with the supposed undefined behavior, but I think you agree that C and assembly files could be set up such that the resulting binary data is identical. In which case it is bogus to call one satisfactory, but not the other. > However, there are still a couple of issued not correctly solved by v8 > of the series. For starters: > > apply_alternatives((struct alt_instr *)alt_instructions_, >(struct alt_instr *)alt_instructions_end_); > > I can see how the pointers comparisons in apply_alternatives could be > considered wrong given the way the pointers are initialized: > > for ( a = base = start; a < end; a++ ) > { > > start
Re: [Xen-devel] [PATCH v4 10/14] argo: implement the notify op
On Thu, Jan 17, 2019 at 01:44:32PM -0800, Christopher Clark wrote: > On Thu, Jan 17, 2019 at 3:12 AM Roger Pau Monné wrote: > > > > On Wed, Jan 16, 2019 at 10:54:48PM -0800, Christopher Clark wrote: > > > On Tue, Jan 15, 2019 at 8:19 AM Roger Pau Monné > > > wrote: > > > > > > > > On Tue, Jan 15, 2019 at 01:27:42AM -0800, Christopher Clark wrote: > > > > > diff --git a/xen/include/public/argo.h b/xen/include/public/argo.h > > > > > index c12a50f..d2cb594 100644 > > > > > --- a/xen/include/public/argo.h > > > > > +++ b/xen/include/public/argo.h > > > > > @@ -123,6 +123,42 @@ typedef struct xen_argo_unregister_ring > > > > > /* Messages on the ring are padded to a multiple of this size. */ > > > > > #define XEN_ARGO_MSG_SLOT_SIZE 0x10 > > > > > > > > > > +/* > > > > > + * Notify flags > > > > > + */ > > > > > +/* Ring is empty */ > > > > > +#define XEN_ARGO_RING_DATA_F_EMPTY (1U << 0) > > > > > +/* Ring exists */ > > > > > +#define XEN_ARGO_RING_DATA_F_EXISTS (1U << 1) > > > > > +/* Pending interrupt exists. Do not rely on this field - for > > > > > profiling only */ > > > > > +#define XEN_ARGO_RING_DATA_F_PENDING (1U << 2) > > > > Regarding this flag, I've just noticed while looking at the code that > > it doesn't seem to relate to interrupts? > > It might not seem that way, but I think it does, because it indicates > that the hypervisor has just queued up a signal (via VIRQ) for later: > the logic in fill_ring_data has observed that there wasn't enough > space available in the ring for the requested space_required supplied > in the notify call, so it has added a new entry to the ring's > pending_ent list, which will cause a signal to be triggered to the > domain (ie. a VIRQ) later when enough space has been observed as being > available. Oh, I think I was getting confused by the wording of the comment, here "pending interrupt" means that the caller should expect an interrupt at some point in the future when there's enough free space on the ring? To me "pending interrupt" means there's an interrupt set by the hypervisor which has not yet been serviced by the caller. > Now, the "len" value stored in that pending_ent can be changed later, > depending on the size of messages that the domain attempts to send to > the same ring in the meantime, which I think is why the comment notes > not to depend upon that flag. > > > From it's usage in fill_ring_data I would write the following > > description: > > > > "Likely not enough space to queue a message of `space_required` > > size." > > > > And then XEN_ARGO_RING_DATA_F_PENDING is completely orthogonal to > > XEN_ARGO_RING_DATA_F_SUFFICIENT, at which point having only one of > > those would be enough? > > Given the above, where I do think that the PENDING flag is an > indicator of queued interrupt, I think there's some merit to keeping > them separate, rather than committing to the client that it is always > one or the other. It actually looks like the call to pending_requeue > is ignoring the potential for an error value (eg ENOSPC or ENOMEM) > there, where the flag should not be set, and possibly the errno should > be returned to the caller. Yes, you should propagate the errors from pending_requeue to the caller. > > AFAICT you cannot get a xen_argo_ring_data_ent_t with both > > XEN_ARGO_RING_DATA_F_PENDING and XEN_ARGO_RING_DATA_F_SUFFICIENT set > > at the same time? > > right, but there is a case where you can get one with neither bit set. Yes, that's right. But you would then get the XEN_ARGO_RING_DATA_F_EMSGSIZE flag set or the ring simply don't exist. > It looks a bit clearer for the caller to have the explicit separate > bits because it can avoid having to check a third flag first to see > how to interpret a combined one. There are three possible situations, which are mutually exclusive: 1. Message is bigger than the max message size supported by the ring: set EMSGSIZE 2. Message fits based on the current available space on the ring: don't set any flags. 3. Message doesn't fit based on the current available space on the ring: set NOTIFY. So that would leave the following set of flags: /* Ring is empty. */ #define XEN_ARGO_RING_EMPTY (1U << 0) /* Ring exists. */ #define XEN_ARGO_RING_EXISTS (1U << 1) /* * Not enough ring space available for the requested size, caller set * to receive a notification via VIRQ_ARGO when enough free space * might be available. */ #define XEN_ARGO_RING_NOTIFY (1U << 2) /* Requested size exceeds maximum ring message size. */ #define XEN_ARGO_RING_EMSGSIZE(1U << 3) /* Ring is shared, not unicast. */ #define XEN_ARGO_RING_SHARED (1U << 4) Note that I've also removed the _DATA_F_, I think it's not specially helpful, and shorter names are easier to read. I think the above is clearer and should be able to convey the same set of information using one flag less, which is always better IMO. That being set I don't know the users of this interface anyway, so if you
Re: [Xen-devel] [PATCH v2] drm/xen-front: Make shmem backed display buffer coherent
On 1/17/19 11:18 AM, Christoph Hellwig wrote: > On Wed, Jan 16, 2019 at 06:43:29AM +, Oleksandr Andrushchenko wrote: >>> This whole issue keeps getting more and more confusing. >> Well, I don't really do DMA here, but instead the buffers in >> question are shared with other Xen domain, so effectively it >> could be thought of some sort of DMA here, where the "device" is >> that remote domain. If the buffers are not flushed then the >> remote part sees some inconsistency which in my case results >> in artifacts on screen while displaying the buffers. >> When buffers are allocated via DMA API then there are no artifacts; >> if buffers are allocated with shmem + DMA mapping then there are no >> artifacts as well. >> The only offending use-case is when I use shmem backed buffers, >> but do not flush them > The right answer would be to implement cache maintainance hooks for > this case in the Xen arch code. These would basically look the same > as the low-level cache maintainance used by the DMA ops, but without > going through the DMA mapping layer, in fact they should probably > reuse the same low-level assembly routines. > > I don't think this is the first usage of such Xen buffer sharing, so > what do the other users do? I'll have to get even deeper into it. Initially I looked at the code, but didn't find anything useful. Or maybe I have just overlooked obvious things there Thank you, Oleksandr ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [qemu-mainline bisection] complete build-i386
branch xen-unstable xenbranch xen-unstable job build-i386 testid xen-build Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://git.qemu.org/qemu.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: qemuu git://git.qemu.org/qemu.git Bug introduced: b6af8926fb858c4f1426e5acb2cfc1f0580ec98a Bug not present: d4683cf952d3bdcbcbfcfd982d77cb6b02041040 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/132052/ commit b6af8926fb858c4f1426e5acb2cfc1f0580ec98a Author: Paul Durrant Date: Tue Jan 8 14:48:59 2019 + xen: add implementations of xen-block connect and disconnect functions... ...and wire in the dataplane. This patch adds the remaining code to make the xen-block XenDevice functional. The parameters that a block frontend expects to find are populated in the backend xenstore area, and the 'ring-ref' and 'event-channel' values specified in the frontend xenstore area are mapped/bound and used to set up the dataplane. Signed-off-by: Paul Durrant Reviewed-by: Anthony Perard Signed-off-by: Anthony PERARD For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/qemu-mainline/build-i386.xen-build.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/qemu-mainline/build-i386.xen-build --summary-out=tmp/132052.bisection-summary --basis-template=131842 --blessings=real,real-bisect qemu-mainline build-i386 xen-build Searching for failure / basis pass: 131980 fail [host=debina1] / 131842 [host=baroque1] 131801 ok. Failure / basis pass flights: 131980 / 131801 (tree with no url: minios) (tree with no url: ovmf) (tree with no url: seabios) Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://git.qemu.org/qemu.git Tree: xen git://xenbits.xen.org/xen.git Latest d0d8ad39ecb51cd7497cd524484fe09f50876798 4b9f0b0f7c84eea2dfb0d5be3e0254bc91319dbc 93a62c544e20ba9e141e411bbaae3d65259d13a3 Basis pass d0d8ad39ecb51cd7497cd524484fe09f50876798 c102d9471f8f02d9fbea72ec4505d7089173f470 a5b0eb363694e7e15405f0b3fc5fb6fab79df1db Generating revisions with ./adhoc-revtuple-generator git://xenbits.xen.org/qemu-xen-traditional.git#d0d8ad39ecb51cd7497cd524484fe09f50876798-d0d8ad39ecb51cd7497cd524484fe09f50876798 git://git.qemu.org/qemu.git#c102d9471f8f02d9fbea72ec4505d7089173f470-4b9f0b0f7c84eea2dfb0d5be3e0254bc91319dbc git://xenbits.xen.org/xen.git#a5b0eb363694e7e15405f0b3fc5fb6fab79df1db-93a62c544e20ba9e141e411bbaae3d65259d13a3 Loaded 5051 nodes in revision graph Searching for test results: 131801 pass d0d8ad39ecb51cd7497cd524484fe09f50876798 c102d9471f8f02d9fbea72ec4505d7089173f470 a5b0eb363694e7e15405f0b3fc5fb6fab79df1db 131842 [host=baroque1] 131980 fail d0d8ad39ecb51cd7497cd524484fe09f50876798 4b9f0b0f7c84eea2dfb0d5be3e0254bc91319dbc 93a62c544e20ba9e141e411bbaae3d65259d13a3 131963 [host=debina0] 132047 pass d0d8ad39ecb51cd7497cd524484fe09f50876798 d4683cf952d3bdcbcbfcfd982d77cb6b02041040 93a62c544e20ba9e141e411bbaae3d65259d13a3 132028 [host=debina0] 132029 [host=debina0] 132048 fail d0d8ad39ecb51cd7497cd524484fe09f50876798 b6af8926fb858c4f1426e5acb2cfc1f0580ec98a 93a62c544e20ba9e141e411bbaae3d65259d13a3 132030 [host=debina0] 132031 pass d0d8ad39ecb51cd7497cd524484fe09f50876798 c102d9471f8f02d9fbea72ec4505d7089173f470 a5b0eb363694e7e15405f0b3fc5fb6fab79df1db 132049 pass d0d8ad39ecb51cd7497cd524484fe09f50876798 d4683cf952d3bdcbcbfcfd982d77cb6b02041040 93a62c544e20ba9e141e411bbaae3d65259d13a3 132033 fail d0d8ad39ecb51cd7497cd524484fe09f50876798 4b9f0b0f7c84eea2dfb0d5be3e0254bc91319dbc 93a62c544e20ba9e141e411bbaae3d65259d13a3 132050 fail d0d8ad39ecb51cd7497cd524484fe09f50876798 b6af8926fb858c4f1426e5acb2cfc1f0580ec98a 93a62c544e20ba9e141e411bbaae3d65259d13a3 132035 fail d0d8ad39ecb51cd7497cd524484fe09f50876798 89bd861c2b470e3fb45596945509079c72af3ac2 93a62c544e20ba9e141e411bbaae3d65259d13a3 132036 pass d0d8ad39ecb51cd7497cd524484fe09f50876798 f3b604e31d8450e42b93cb9042341c8b267cc22c 93a62c544e20ba9e141e411bbaae3d65259d13a3 132051 pass d0d8ad39ecb51cd7497cd524484fe09f50876798 d4683cf952d3bdcbcbfcfd982d77cb6b02041040 93a62c544e20ba9e141e411bbaae3d65259d13a3 132037 pass d0d8ad39ecb51cd7497cd524484fe09f50876798 27df21ca3886fff4dd3d70e515517667963a52f1 93a62c544e20ba9e141e411bbaae3d65259d13a3 132038 pass d0d8ad39ecb51cd7497cd524484fe09f50876798 ce2eefd7c21697fee87a0686353de881081d22c6 93a62c544e20ba9e141e411bbaae3d65259d13a3 132039 pass d0d8ad39ecb51cd7497cd524484fe09f50876798 d63a6af935327945eaf687da6119fbabe03be3f9 93a62c544e20ba9e141e411bbaae3d65259d13a3 132026 [host=debina0] 132052 fail d0d8ad39ecb51cd7497cd524484fe09f50876798 b6af8926fb858c4f1426e5acb2cfc1f0580ec98a
Re: [Xen-devel] [PATCH RFC for next] xen: make grant table configurable
On Thu, Jan 17, 2019 at 08:19:28PM +, Andrew Cooper wrote: > On 17/01/2019 12:30, Wei Liu wrote: > > Introduce CONFIG_GRANT_TABLE. Provide stubs and make sure x86 and arm > > hypervisors build with grant table disabled. > > > > Signed-off-by: Wei Liu > > --- > > I did this when I worked on splitting PV and HVM and thought this > > might be useful and it was simple enough to get done. > > > > RFC because I can envisage some configurations in the distant future > > do away with grant table and event channel. There is small a benefit > > to consider accepting this patch now so that Gitlab's randconf build > > can start testing with grant table turned off right away. > > --- > > xen/arch/arm/setup.c | 3 ++- > > xen/arch/arm/traps.c | 2 ++ > > xen/arch/x86/hvm/Makefile | 2 +- > > xen/arch/x86/hvm/hypercall.c | 4 > > xen/arch/x86/hypercall.c | 2 ++ > > xen/arch/x86/pv/Makefile | 2 +- > > xen/arch/x86/pv/hypercall.c | 2 ++ > > xen/arch/x86/setup.c | 6 +++-- > > xen/common/Kconfig| 11 + > > xen/common/Makefile | 2 +- > > xen/include/asm-arm/grant_table.h | 4 +++- > > xen/include/xen/grant_table.h | 49 > > +-- > > 12 files changed, 80 insertions(+), 9 deletions(-) > > > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > > index 444857a967..3cd3513928 100644 > > --- a/xen/arch/arm/setup.c > > +++ b/xen/arch/arm/setup.c > > @@ -740,7 +740,8 @@ void __init start_xen(unsigned long boot_phys_offset, > > .flags = XEN_DOMCTL_CDF_hvm_guest | XEN_DOMCTL_CDF_hap, > > .max_evtchn_port = -1, > > .max_grant_frames = gnttab_dom0_frames(), > > -.max_maptrack_frames = opt_max_maptrack_frames, > > +.max_maptrack_frames = IS_ENABLED(CONFIG_GRANT_TABLE) ? > > + opt_max_maptrack_frames : 0, > > You can remove this conditional logic by adding #define > opt_max_grant_frames 0 in the stubs in grant_table.h > Ack. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC for next] xen: make grant table configurable
On Fri, Jan 18, 2019 at 12:48:42AM -0700, Jan Beulich wrote: > >>> On 17.01.19 at 13:30, wrote: > > --- a/xen/common/Kconfig > > +++ b/xen/common/Kconfig > > @@ -11,6 +11,17 @@ config COMPAT > > config CORE_PARKING > > bool > > > > +config GRANT_TABLE > > + bool "Grant table support" > > + default y > > + ---help--- > > + Grant table provides a generic mechanism to memory sharing > > + between domains. This shared memory interface underpins the > > + split device drivers for block and network IO in a classic > > + Xen setup. > > I can see reasons why one may want this (along the lines of SILO > mode), but it'll produce an entirely ABI-incompatible hypervisor. > Most front-/backend driver pairs won't work without it. I'm not > sure the tool stack is prepared to act on such a hypervisor. I'm > pretty sure at least XenoLinux derivatives BUG() upon error > returns from certain gnttab hypercalls. > > As such as well as to be within our general line of what Kconfig > options we permit, I think for the time being the prompt needs > to depend on EXPERT = "y". Good point. I will do that. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 19/21] treewide: add checks for the return value of memblock_alloc*()
On Wed, Jan 16, 2019 at 03:44:19PM +0200, Mike Rapoport wrote: > Add check for the return value of memblock_alloc*() functions and call > panic() in case of error. > The panic message repeats the one used by panicing memblock allocators with > adjustment of parameters to include only relevant ones. > > The replacement was mostly automated with semantic patches like the one > below with manual massaging of format strings. > > @@ > expression ptr, size, align; > @@ > ptr = memblock_alloc(size, align); > + if (!ptr) > + panic("%s: Failed to allocate %lu bytes align=0x%lx\n", __func__, > size, align); > > Signed-off-by: Mike Rapoport ... > diff --git a/arch/s390/numa/toptree.c b/arch/s390/numa/toptree.c > index 71a608c..0118c77 100644 > --- a/arch/s390/numa/toptree.c > +++ b/arch/s390/numa/toptree.c > @@ -31,10 +31,14 @@ struct toptree __ref *toptree_alloc(int level, int id) > { > struct toptree *res; > > - if (slab_is_available()) > + if (slab_is_available()) { > res = kzalloc(sizeof(*res), GFP_KERNEL); > - else > + } else { > res = memblock_alloc(sizeof(*res), 8); > + if (!res) > + panic("%s: Failed to allocate %zu bytes align=0x%x\n", > + __func__, sizeof(*res), 8); > + } > if (!res) > return res; Please remove this hunk, since the code _should_ be able to handle allocation failures anyway (see end of quoted code). Otherwise for the s390 bits: Acked-by: Heiko Carstens ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 21/21] memblock: drop memblock_alloc_*_nopanic() variants
On Wed, Jan 16, 2019 at 03:44:21PM +0200, Mike Rapoport wrote: > As all the memblock allocation functions return NULL in case of error > rather than panic(), the duplicates with _nopanic suffix can be removed. > > Signed-off-by: Mike Rapoport > --- > arch/arc/kernel/unwind.c | 3 +-- > arch/sh/mm/init.c | 2 +- > arch/x86/kernel/setup_percpu.c | 10 +- > arch/x86/mm/kasan_init_64.c| 14 -- > drivers/firmware/memmap.c | 2 +- > drivers/usb/early/xhci-dbc.c | 2 +- > include/linux/memblock.h | 35 --- > kernel/dma/swiotlb.c | 2 +- > kernel/printk/printk.c | 17 +++-- > mm/memblock.c | 35 --- > mm/page_alloc.c| 10 +- > mm/page_ext.c | 2 +- > mm/percpu.c| 11 --- > mm/sparse.c| 6 ++ > 14 files changed, 37 insertions(+), 114 deletions(-) Acked-by: Greg Kroah-Hartman ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel