[linux-linus test] 180917: regressions - FAIL
flight 180917 linux-linus real [real] flight 180923 linux-linus real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/180917/ http://logs.test-lab.xenproject.org/osstest/logs/180923/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-credit1 8 xen-boot fail REGR. vs. 180278 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 8 xen-boot fail like 180278 test-armhf-armhf-examine 8 reboot fail like 180278 test-armhf-armhf-xl-arndale 8 xen-boot fail like 180278 test-armhf-armhf-libvirt-qcow2 8 xen-bootfail like 180278 test-armhf-armhf-libvirt-raw 8 xen-boot fail like 180278 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180278 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180278 test-armhf-armhf-xl-vhd 8 xen-boot fail like 180278 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180278 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180278 test-armhf-armhf-xl-multivcpu 8 xen-boot fail like 180278 test-armhf-armhf-xl-credit2 8 xen-boot fail like 180278 test-armhf-armhf-xl-rtds 8 xen-boot fail like 180278 test-armhf-armhf-xl 8 xen-boot fail like 180278 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180278 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass version targeted for testing: linuxae8373a5add4ea39f032563cf12a02946d1e3546 baseline version: linux6c538e1adbfc696ac4747fb10d63e704344f763d Last test of basis 180278 2023-04-16 19:41:46 Z 37 days Failing since180281 2023-04-17 06:24:36 Z 36 days 68 attempts Testing same since 180907 2023-05-23 01:11:53 Z1 days2 attempts 2480 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-xl pass
Re: PVH Dom0 related UART failure
On Tue, 23 May 2023, Jan Beulich wrote: > On 23.05.2023 00:20, Stefano Stabellini wrote: > > On Sat, 20 May 2023, Roger Pau Monné wrote: > >> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c > >> index ec2e978a4e6b..0ff8e940fa8d 100644 > >> --- a/xen/drivers/vpci/header.c > >> +++ b/xen/drivers/vpci/header.c > >> @@ -289,6 +289,13 @@ static int modify_bars(const struct pci_dev *pdev, > >> uint16_t cmd, bool rom_only) > >> */ > >> for_each_pdev ( pdev->domain, tmp ) > >> { > >> +if ( !tmp->vpci ) > >> +{ > >> +printk(XENLOG_G_WARNING "%pp: not handled by vPCI for %pd\n", > >> + >sbdf, pdev->domain); > >> +continue; > >> +} > >> + > >> if ( tmp == pdev ) > >> { > >> /* > >> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c > >> index 652807a4a454..0baef3a8d3a1 100644 > >> --- a/xen/drivers/vpci/vpci.c > >> +++ b/xen/drivers/vpci/vpci.c > >> @@ -72,7 +72,12 @@ int vpci_add_handlers(struct pci_dev *pdev) > >> unsigned int i; > >> int rc = 0; > >> > >> -if ( !has_vpci(pdev->domain) ) > >> +if ( !has_vpci(pdev->domain) || > >> + /* > >> + * Ignore RO and hidden devices, those are in use by Xen and vPCI > >> + * won't work on them. > >> + */ > >> + pci_get_pdev(dom_xen, pdev->sbdf) ) > >> return 0; > >> > >> /* We should not get here twice for the same device. */ > > > > > > Now this patch works! Thank you!! :-) > > > > You can check the full logs here > > https://gitlab.com/xen-project/people/sstabellini/xen/-/jobs/4329259080 > > > > Is the patch ready to be upstreamed aside from the commit message? > > I don't think so. vPCI ought to work on "r/o" devices. Out of curiosity, > have you also tried my (hackish and hence RFC) patch [1]? > > [1] https://lists.xen.org/archives/html/xen-devel/2021-08/msg01489.html I don't know the code well enough to discuss what is the best solution. I'll let you and Roger figure it out. I would only kindly request to solve this in few days so that we can enable the real hardware PVH test in gitlab-ci as soon as possible. I think it is critical as it will allow us to catch many real issues going forward. For sure I can test your patch. BTW it is also really easy for you to do it your simply by pushing a branch to a repo on gitlab-ci and watch for the results. If you are interested let me know I can give you a tutorial, you just need to create a repo, and register the gitlab runner and voila'. This is the outcome: https://gitlab.com/xen-project/people/sstabellini/xen/-/pipelines/876808194 (XEN) PCI add device :00:00.0 (XEN) PCI add device :00:00.2 (XEN) PCI add device :00:01.0 (XEN) PCI add device :00:02.0 (XEN) Assertion 'd == dom_xen && system_state < SYS_STATE_active' failed at drivers/vpci/header.c:313 (XEN) [ Xen-4.18-unstable x86_64 debug=y Tainted: C] (XEN) CPU:14 (XEN) RIP:e008:[] drivers/vpci/header.c#modify_bars+0x3ba/0x4a3 (XEN) RFLAGS: 00010202 CONTEXT: hypervisor (d0v0) (XEN) rax: 830390164000 rbx: 83038bd8a7f0 rcx: 0010 (XEN) rdx: 83038e3b7fff rsi: 83038e3c83a0 rdi: 83038e3c8398 (XEN) rbp: 83038e3b7c08 rsp: 83038e3b7b98 r8: 0001 (XEN) r9: 83038dcfa000 r10: 000e r11: (XEN) r12: 0007 r13: 000dcc03 r14: 83039016ad10 (XEN) r15: 000dcc00 cr0: 8005003b cr4: 00f506e0 (XEN) cr3: 00038ddfe000 cr2: 88814e3ff000 (XEN) fsb: gsb: 888149a0 gss: (XEN) ds: es: fs: gs: ss: cs: e008 (XEN) Xen code around (drivers/vpci/header.c#modify_bars+0x3ba/0x4a3): (XEN) 3d c4 d1 37 00 02 76 c0 <0f> 0b 4c 8b 75 b0 0f ae e8 48 83 7d 98 00 74 2b (XEN) Xen stack trace from rsp=83038e3b7b98: (XEN)0002 830390150230 830390164000 830390164158 (XEN)830390150230 00ff8303 83038dcf8b00 0003 (XEN)0206 83038bd8c010 0002 (XEN)0002 0004 83038e3b7c18 82d040268909 (XEN)83038e3b7ca0 82d040267998 00118e3b7ca0 00117803 (XEN)0004 830390150230 (XEN)00040002 (XEN)0002 0004 (XEN)82d04041df40 83038e3b7cd0 82d0402cb649 001140332c9e (XEN)83038e3b7d88 83038dd094a0 83038e3b7d30 (XEN)82d0402caa72 83038e3b7ce0 0cfc0002 (XEN)0002 83038dd094a0 83038e3b7d88 (XEN)0001
[xen-4.16-testing test] 180918: tolerable trouble: fail/pass/starved - PUSHED
flight 180918 xen-4.16-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/180918/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 180478 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180478 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 180478 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180478 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180478 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 180478 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180478 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180478 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180478 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 180478 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 180478 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180478 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 3 hosts-allocate starved n/a test-arm64-arm64-xl 3 hosts-allocate starved n/a test-arm64-arm64-xl-credit1 3 hosts-allocate starved n/a test-arm64-arm64-xl-thunderx 3 hosts-allocate starved n/a test-arm64-arm64-xl-vhd 3 hosts-allocate starved n/a test-arm64-arm64-xl-xsm 3 hosts-allocate starved n/a test-arm64-arm64-libvirt-raw 3 hosts-allocate starved n/a test-arm64-arm64-libvirt-xsm 3 hosts-allocate starved n/a version targeted for testing: xen b0806d84d48d983d40a29534e663652887287a78 baseline version: xen 7251cea957cbe4a0772651a4ab110ed76f689f96 Last test of basis 180478 2023-04-29 03:21:22 Z 24 days Testing same since 180918 2023-05-23 13:08:46 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Dario Faggioli Jan Beulich Jason Andryuk Juergen Gross Marek Marczykowski-Górecki Olaf Hering Roger Pau Monné Stewart Hildebrand Yann Dirson jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64 pass build-arm64 pass build-armhf pass build-i386
[xen-unstable test] 180913: tolerable FAIL
flight 180913 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/180913/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemuu-win7-amd64 12 windows-install fail in 180900 pass in 180913 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-installfail pass in 180900 test-amd64-i386-libvirt-raw 7 xen-installfail pass in 180900 test-amd64-amd64-libvirt-vhd 19 guest-start/debian.repeat fail pass in 180900 test-amd64-i386-xl-vhd 22 guest-start.2 fail pass in 180900 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stop fail blocked in 180900 test-amd64-i386-libvirt-raw 14 migrate-support-check fail in 180900 never pass test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180900 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180900 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180900 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 180900 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 180900 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 180900 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 180900 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 180900 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180900 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180900 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180900 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass
Re: [PATCH] [v2] x86: xen: add missing prototypes
On 5/23/23 4:37 PM, Arnd Bergmann wrote: On Sat, May 20, 2023, at 00:24, Boris Ostrovsky wrote: On 5/19/23 5:28 AM, Arnd Bergmann wrote: diff --git a/arch/x86/xen/smp.h b/arch/x86/xen/smp.h index 22fb982ff971..81a7821dd07f 100644 --- a/arch/x86/xen/smp.h +++ b/arch/x86/xen/smp.h @@ -1,7 +1,11 @@ /* SPDX-License-Identifier: GPL-2.0 */ #ifndef _XEN_SMP_H +void asm_cpu_bringup_and_idle(void); +asmlinkage void cpu_bringup_and_idle(void); These can go under CONFIG_SMP Not sure if there is much point for the second one, since it's only called from assembler, so the #else path is never seen, but I can do that for consistency if you like. I generally prefer to avoid the extra #if checks when there is no strict need for an empty stub. Do we need the empty stubs? Neither of these should be referenced if !SMP (or more precisely if !CONFIG_XEN_PV_SMP) I guess you also want me to add the empty stubs for the other functions that only have a bit in #ifdef but not #else then? +void xen_force_evtchn_callback(void); These ... +pteval_t xen_pte_val(pte_t pte); +pgdval_t xen_pgd_val(pgd_t pgd); +pte_t xen_make_pte(pteval_t pte); +pgd_t xen_make_pgd(pgdval_t pgd); +pmdval_t xen_pmd_val(pmd_t pmd); +pmd_t xen_make_pmd(pmdval_t pmd); +pudval_t xen_pud_val(pud_t pud); +pud_t xen_make_pud(pudval_t pud); +p4dval_t xen_p4d_val(p4d_t p4d); +p4d_t xen_make_p4d(p4dval_t p4d); +pte_t xen_make_pte_init(pteval_t pte); +void xen_start_kernel(struct start_info *si); ... should go under '#ifdef CONFIG_XEN_PV' What should the stubs do here? I guess just return the unmodified value? Same here -- these should only be referenced if CONFIG_XEN_PV is defined which is why I suggested moving them under ifdef. -boris
[PATCH] [v3] x86: xen: add missing prototypes
From: Arnd Bergmann These function are all called from assembler files, or from inline assembler, so there is no immediate need for a prototype in a header, but if -Wmissing-prototypes is enabled, the compiler warns about them: arch/x86/xen/efi.c:130:13: error: no previous prototype for 'xen_efi_init' [-Werror=missing-prototypes] arch/x86/platform/pvh/enlighten.c:120:13: error: no previous prototype for 'xen_prepare_pvh' [-Werror=missing-prototypes] arch/x86/xen/mmu_pv.c:358:20: error: no previous prototype for 'xen_pte_val' [-Werror=missing-prototypes] arch/x86/xen/mmu_pv.c:366:20: error: no previous prototype for 'xen_pgd_val' [-Werror=missing-prototypes] arch/x86/xen/mmu_pv.c:372:17: error: no previous prototype for 'xen_make_pte' [-Werror=missing-prototypes] arch/x86/xen/mmu_pv.c:380:17: error: no previous prototype for 'xen_make_pgd' [-Werror=missing-prototypes] arch/x86/xen/mmu_pv.c:387:20: error: no previous prototype for 'xen_pmd_val' [-Werror=missing-prototypes] arch/x86/xen/mmu_pv.c:425:17: error: no previous prototype for 'xen_make_pmd' [-Werror=missing-prototypes] arch/x86/xen/mmu_pv.c:432:20: error: no previous prototype for 'xen_pud_val' [-Werror=missing-prototypes] arch/x86/xen/mmu_pv.c:438:17: error: no previous prototype for 'xen_make_pud' [-Werror=missing-prototypes] arch/x86/xen/mmu_pv.c:522:20: error: no previous prototype for 'xen_p4d_val' [-Werror=missing-prototypes] arch/x86/xen/mmu_pv.c:528:17: error: no previous prototype for 'xen_make_p4d' [-Werror=missing-prototypes] arch/x86/xen/mmu_pv.c:1442:17: error: no previous prototype for 'xen_make_pte_init' [-Werror=missing-prototypes] arch/x86/xen/enlighten_pv.c:1233:34: error: no previous prototype for 'xen_start_kernel' [-Werror=missing-prototypes] arch/x86/xen/irq.c:22:14: error: no previous prototype for 'xen_force_evtchn_callback' [-Werror=missing-prototypes] arch/x86/entry/common.c:302:24: error: no previous prototype for 'xen_pv_evtchn_do_upcall' [-Werror=missing-prototypes] Declare all of them in an appropriate header file to avoid the warnings. For consistency, also move the asm_cpu_bringup_and_idle() declaration out of smp_pv.c. Signed-off-by: Arnd Bergmann --- v3: move declations of conditional function in an #ifdef with a stub v2: fix up changelog --- arch/x86/xen/efi.c | 2 ++ arch/x86/xen/smp.h | 11 +++ arch/x86/xen/smp_pv.c | 1 - arch/x86/xen/xen-ops.h | 26 ++ include/xen/xen.h | 3 +++ 5 files changed, 42 insertions(+), 1 deletion(-) diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c index 7d7ffb9c826a..863d0d6b3edc 100644 --- a/arch/x86/xen/efi.c +++ b/arch/x86/xen/efi.c @@ -16,6 +16,8 @@ #include #include +#include "xen-ops.h" + static efi_char16_t vendor[100] __initdata; static efi_system_table_t efi_systab_xen __initdata = { diff --git a/arch/x86/xen/smp.h b/arch/x86/xen/smp.h index 22fb982ff971..9367502281dc 100644 --- a/arch/x86/xen/smp.h +++ b/arch/x86/xen/smp.h @@ -2,6 +2,10 @@ #ifndef _XEN_SMP_H #ifdef CONFIG_SMP + +void asm_cpu_bringup_and_idle(void); +asmlinkage void cpu_bringup_and_idle(void); + extern void xen_send_IPI_mask(const struct cpumask *mask, int vector); extern void xen_send_IPI_mask_allbutself(const struct cpumask *mask, @@ -29,6 +33,13 @@ struct xen_common_irq { }; #else /* CONFIG_SMP */ +static inline void asm_cpu_bringup_and_idle(void) +{ +} +static inline void cpu_bringup_and_idle(void) +{ +} + static inline int xen_smp_intr_init(unsigned int cpu) { return 0; diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c index a92e8002b5cf..d5ae5de2daa2 100644 --- a/arch/x86/xen/smp_pv.c +++ b/arch/x86/xen/smp_pv.c @@ -55,7 +55,6 @@ static DEFINE_PER_CPU(struct xen_common_irq, xen_irq_work) = { .irq = -1 }; static DEFINE_PER_CPU(struct xen_common_irq, xen_pmu_irq) = { .irq = -1 }; static irqreturn_t xen_irq_work_interrupt(int irq, void *dev_id); -void asm_cpu_bringup_and_idle(void); static void cpu_bringup(void) { diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h index 6d7f6318fc07..eb4cb30570c7 100644 --- a/arch/x86/xen/xen-ops.h +++ b/arch/x86/xen/xen-ops.h @@ -146,12 +146,38 @@ int xen_cpuhp_setup(int (*cpu_up_prepare_cb)(unsigned int), void xen_pin_vcpu(int cpu); void xen_emergency_restart(void); +void xen_force_evtchn_callback(void); + #ifdef CONFIG_XEN_PV void xen_pv_pre_suspend(void); void xen_pv_post_suspend(int suspend_cancelled); +pteval_t xen_pte_val(pte_t pte); +pgdval_t xen_pgd_val(pgd_t pgd); +pmdval_t xen_pmd_val(pmd_t pmd); +pudval_t xen_pud_val(pud_t pud); +p4dval_t xen_p4d_val(p4d_t p4d); +pte_t xen_make_pte(pteval_t pte); +pgd_t xen_make_pgd(pgdval_t pgd); +pmd_t xen_make_pmd(pmdval_t pmd); +pud_t xen_make_pud(pudval_t pud); +p4d_t xen_make_p4d(p4dval_t p4d); +pte_t xen_make_pte_init(pteval_t pte); +void xen_start_kernel(struct start_info *si); #else static inline void xen_pv_pre_suspend(void) {} static inline
Re: [PATCH] [v2] x86: xen: add missing prototypes
On Sat, May 20, 2023, at 00:24, Boris Ostrovsky wrote: > On 5/19/23 5:28 AM, Arnd Bergmann wrote: > >> diff --git a/arch/x86/xen/smp.h b/arch/x86/xen/smp.h >> index 22fb982ff971..81a7821dd07f 100644 >> --- a/arch/x86/xen/smp.h >> +++ b/arch/x86/xen/smp.h >> @@ -1,7 +1,11 @@ >> /* SPDX-License-Identifier: GPL-2.0 */ >> #ifndef _XEN_SMP_H >> >> +void asm_cpu_bringup_and_idle(void); >> +asmlinkage void cpu_bringup_and_idle(void); > > These can go under CONFIG_SMP Not sure if there is much point for the second one, since it's only called from assembler, so the #else path is never seen, but I can do that for consistency if you like. I generally prefer to avoid the extra #if checks when there is no strict need for an empty stub. I guess you also want me to add the empty stubs for the other functions that only have a bit in #ifdef but not #else then? >> +void xen_force_evtchn_callback(void); > > These ... > >> +pteval_t xen_pte_val(pte_t pte); >> +pgdval_t xen_pgd_val(pgd_t pgd); >> +pte_t xen_make_pte(pteval_t pte); >> +pgd_t xen_make_pgd(pgdval_t pgd); >> +pmdval_t xen_pmd_val(pmd_t pmd); >> +pmd_t xen_make_pmd(pmdval_t pmd); >> +pudval_t xen_pud_val(pud_t pud); >> +pud_t xen_make_pud(pudval_t pud); >> +p4dval_t xen_p4d_val(p4d_t p4d); >> +p4d_t xen_make_p4d(p4dval_t p4d); >> +pte_t xen_make_pte_init(pteval_t pte); >> +void xen_start_kernel(struct start_info *si); > > > ... should go under '#ifdef CONFIG_XEN_PV' What should the stubs do here? I guess just return the unmodified value? Arnd
Re: [XEN PATCH 13/15] build: fix compile.h compiler version command line
On 23/05/2023 5:38 pm, Anthony PERARD wrote: > CFLAGS is just from Config.mk, instead use the flags used to build > Xen. > > Signed-off-by: Anthony PERARD > --- > > Notes: > I don't know if CFLAGS is even useful there, just --version without the > flags might produce the same result. I can't think of any legitimate reason for CFLAGS to be here. Any compiler which does differ its output based on CLFAGS is probably one we don't want to be using... ~Andrew
[qemu-mainline test] 180916: regressions - FAIL
flight 180916 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/180916/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 6 xen-buildfail REGR. vs. 180691 build-amd64-xsm 6 xen-buildfail REGR. vs. 180691 build-i386-xsm6 xen-buildfail REGR. vs. 180691 build-i3866 xen-buildfail REGR. vs. 180691 build-arm64 6 xen-buildfail REGR. vs. 180691 build-arm64-xsm 6 xen-buildfail REGR. vs. 180691 build-armhf 6 xen-buildfail REGR. vs. 180691 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 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-shadow 1 build-check(1) blocked n/a test-amd64-i386-xl-vhd1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit1 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-thunderx 1 build-check(1) blocked n/a test-arm64-arm64-xl-vhd 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-coresched-i386-xl 1 build-check(1) blocked n/a test-amd64-coresched-amd64-xl 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-shadow1 build-check(1) blocked n/a test-amd64-amd64-xl-rtds 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-amd64-dom0pvh-xl-amd 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-amd64-dom0pvh-xl-intel 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a
[PATCH v2 5/6] block/linux-aio: convert to blk_io_plug_call() API
Stop using the .bdrv_co_io_plug() API because it is not multi-queue block layer friendly. Use the new blk_io_plug_call() API to batch I/O submission instead. Signed-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake --- include/block/raw-aio.h | 7 --- block/file-posix.c | 28 block/linux-aio.c | 41 +++-- 3 files changed, 11 insertions(+), 65 deletions(-) diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h index da60ca13ef..0f63c2800c 100644 --- a/include/block/raw-aio.h +++ b/include/block/raw-aio.h @@ -62,13 +62,6 @@ int coroutine_fn laio_co_submit(int fd, uint64_t offset, QEMUIOVector *qiov, void laio_detach_aio_context(LinuxAioState *s, AioContext *old_context); void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context); - -/* - * laio_io_plug/unplug work in the thread's current AioContext, therefore the - * caller must ensure that they are paired in the same IOThread. - */ -void laio_io_plug(void); -void laio_io_unplug(uint64_t dev_max_batch); #endif /* io_uring.c - Linux io_uring implementation */ #ifdef CONFIG_LINUX_IO_URING diff --git a/block/file-posix.c b/block/file-posix.c index 7baa8491dd..ac1ed54811 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -2550,26 +2550,6 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, int64_t offset, return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_WRITE); } -static void coroutine_fn raw_co_io_plug(BlockDriverState *bs) -{ -BDRVRawState __attribute__((unused)) *s = bs->opaque; -#ifdef CONFIG_LINUX_AIO -if (s->use_linux_aio) { -laio_io_plug(); -} -#endif -} - -static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs) -{ -BDRVRawState __attribute__((unused)) *s = bs->opaque; -#ifdef CONFIG_LINUX_AIO -if (s->use_linux_aio) { -laio_io_unplug(s->aio_max_batch); -} -#endif -} - static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs) { BDRVRawState *s = bs->opaque; @@ -3914,8 +3894,6 @@ BlockDriver bdrv_file = { .bdrv_co_copy_range_from = raw_co_copy_range_from, .bdrv_co_copy_range_to = raw_co_copy_range_to, .bdrv_refresh_limits = raw_refresh_limits, -.bdrv_co_io_plug= raw_co_io_plug, -.bdrv_co_io_unplug = raw_co_io_unplug, .bdrv_attach_aio_context = raw_aio_attach_aio_context, .bdrv_co_truncate = raw_co_truncate, @@ -4286,8 +4264,6 @@ static BlockDriver bdrv_host_device = { .bdrv_co_copy_range_from = raw_co_copy_range_from, .bdrv_co_copy_range_to = raw_co_copy_range_to, .bdrv_refresh_limits = raw_refresh_limits, -.bdrv_co_io_plug= raw_co_io_plug, -.bdrv_co_io_unplug = raw_co_io_unplug, .bdrv_attach_aio_context = raw_aio_attach_aio_context, .bdrv_co_truncate = raw_co_truncate, @@ -4424,8 +4400,6 @@ static BlockDriver bdrv_host_cdrom = { .bdrv_co_pwritev= raw_co_pwritev, .bdrv_co_flush_to_disk = raw_co_flush_to_disk, .bdrv_refresh_limits= cdrom_refresh_limits, -.bdrv_co_io_plug= raw_co_io_plug, -.bdrv_co_io_unplug = raw_co_io_unplug, .bdrv_attach_aio_context = raw_aio_attach_aio_context, .bdrv_co_truncate = raw_co_truncate, @@ -4552,8 +4526,6 @@ static BlockDriver bdrv_host_cdrom = { .bdrv_co_pwritev= raw_co_pwritev, .bdrv_co_flush_to_disk = raw_co_flush_to_disk, .bdrv_refresh_limits= cdrom_refresh_limits, -.bdrv_co_io_plug= raw_co_io_plug, -.bdrv_co_io_unplug = raw_co_io_unplug, .bdrv_attach_aio_context = raw_aio_attach_aio_context, .bdrv_co_truncate = raw_co_truncate, diff --git a/block/linux-aio.c b/block/linux-aio.c index 442c86209b..5021aed68f 100644 --- a/block/linux-aio.c +++ b/block/linux-aio.c @@ -15,6 +15,7 @@ #include "qemu/event_notifier.h" #include "qemu/coroutine.h" #include "qapi/error.h" +#include "sysemu/block-backend.h" /* Only used for assertions. */ #include "qemu/coroutine_int.h" @@ -46,7 +47,6 @@ struct qemu_laiocb { }; typedef struct { -int plugged; unsigned int in_queue; unsigned int in_flight; bool blocked; @@ -236,7 +236,7 @@ static void qemu_laio_process_completions_and_submit(LinuxAioState *s) { qemu_laio_process_completions(s); -if (!s->io_q.plugged && !QSIMPLEQ_EMPTY(>io_q.pending)) { +if (!QSIMPLEQ_EMPTY(>io_q.pending)) { ioq_submit(s); } } @@ -277,7 +277,6 @@ static void qemu_laio_poll_ready(EventNotifier *opaque) static void ioq_init(LaioQueue *io_q) { QSIMPLEQ_INIT(_q->pending); -io_q->plugged = 0; io_q->in_queue = 0; io_q->in_flight = 0; io_q->blocked = false; @@ -354,31 +353,11 @@ static uint64_t laio_max_batch(LinuxAioState *s, uint64_t dev_max_batch) return max_batch; } -void laio_io_plug(void) +static void laio_unplug_fn(void *opaque) {
[PATCH v2 6/6] block: remove bdrv_co_io_plug() API
No block driver implements .bdrv_co_io_plug() anymore. Get rid of the function pointers. Signed-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake --- include/block/block-io.h | 3 --- include/block/block_int-common.h | 11 -- block/io.c | 37 3 files changed, 51 deletions(-) diff --git a/include/block/block-io.h b/include/block/block-io.h index a27e471a87..43af816d75 100644 --- a/include/block/block-io.h +++ b/include/block/block-io.h @@ -259,9 +259,6 @@ void coroutine_fn bdrv_co_leave(BlockDriverState *bs, AioContext *old_ctx); AioContext *child_of_bds_get_parent_aio_context(BdrvChild *c); -void coroutine_fn GRAPH_RDLOCK bdrv_co_io_plug(BlockDriverState *bs); -void coroutine_fn GRAPH_RDLOCK bdrv_co_io_unplug(BlockDriverState *bs); - bool coroutine_fn GRAPH_RDLOCK bdrv_co_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name, uint32_t granularity, Error **errp); diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index 6492a1e538..958962aa3a 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -753,11 +753,6 @@ struct BlockDriver { void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_debug_event)( BlockDriverState *bs, BlkdebugEvent event); -/* io queue for linux-aio */ -void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_io_plug)(BlockDriverState *bs); -void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_io_unplug)( -BlockDriverState *bs); - /** * bdrv_drain_begin is called if implemented in the beginning of a * drain operation to drain and stop any internal sources of requests in @@ -1227,12 +1222,6 @@ struct BlockDriverState { unsigned int in_flight; unsigned int serialising_in_flight; -/* - * counter for nested bdrv_io_plug. - * Accessed with atomic ops. - */ -unsigned io_plugged; - /* do we need to tell the quest if we have a volatile write cache? */ int enable_write_cache; diff --git a/block/io.c b/block/io.c index 4d54fda593..56b0c1ce6c 100644 --- a/block/io.c +++ b/block/io.c @@ -3219,43 +3219,6 @@ void *qemu_try_blockalign0(BlockDriverState *bs, size_t size) return mem; } -void coroutine_fn bdrv_co_io_plug(BlockDriverState *bs) -{ -BdrvChild *child; -IO_CODE(); -assert_bdrv_graph_readable(); - -QLIST_FOREACH(child, >children, next) { -bdrv_co_io_plug(child->bs); -} - -if (qatomic_fetch_inc(>io_plugged) == 0) { -BlockDriver *drv = bs->drv; -if (drv && drv->bdrv_co_io_plug) { -drv->bdrv_co_io_plug(bs); -} -} -} - -void coroutine_fn bdrv_co_io_unplug(BlockDriverState *bs) -{ -BdrvChild *child; -IO_CODE(); -assert_bdrv_graph_readable(); - -assert(bs->io_plugged); -if (qatomic_fetch_dec(>io_plugged) == 1) { -BlockDriver *drv = bs->drv; -if (drv && drv->bdrv_co_io_unplug) { -drv->bdrv_co_io_unplug(bs); -} -} - -QLIST_FOREACH(child, >children, next) { -bdrv_co_io_unplug(child->bs); -} -} - /* Helper that undoes bdrv_register_buf() when it fails partway through */ static void GRAPH_RDLOCK bdrv_register_buf_rollback(BlockDriverState *bs, void *host, size_t size, -- 2.40.1
[PATCH v2 3/6] block/blkio: convert to blk_io_plug_call() API
Stop using the .bdrv_co_io_plug() API because it is not multi-queue block layer friendly. Use the new blk_io_plug_call() API to batch I/O submission instead. Signed-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake --- v2 - Add missing #include and fix blkio_unplug_fn() prototype [Stefano] --- block/blkio.c | 43 --- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/block/blkio.c b/block/blkio.c index 0cdc99a729..93c6d20d39 100644 --- a/block/blkio.c +++ b/block/blkio.c @@ -17,6 +17,7 @@ #include "qemu/error-report.h" #include "qapi/qmp/qdict.h" #include "qemu/module.h" +#include "sysemu/block-backend.h" #include "exec/memory.h" /* for ram_block_discard_disable() */ #include "block/block-io.h" @@ -325,16 +326,30 @@ static void blkio_detach_aio_context(BlockDriverState *bs) false, NULL, NULL, NULL, NULL, NULL); } -/* Call with s->blkio_lock held to submit I/O after enqueuing a new request */ -static void blkio_submit_io(BlockDriverState *bs) +/* + * Called by blk_io_unplug() or immediately if not plugged. Called without + * blkio_lock. + */ +static void blkio_unplug_fn(void *opaque) { -if (qatomic_read(>io_plugged) == 0) { -BDRVBlkioState *s = bs->opaque; +BDRVBlkioState *s = opaque; +WITH_QEMU_LOCK_GUARD(>blkio_lock) { blkioq_do_io(s->blkioq, NULL, 0, 0, NULL); } } +/* + * Schedule I/O submission after enqueuing a new request. Called without + * blkio_lock. + */ +static void blkio_submit_io(BlockDriverState *bs) +{ +BDRVBlkioState *s = bs->opaque; + +blk_io_plug_call(blkio_unplug_fn, s); +} + static int coroutine_fn blkio_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes) { @@ -345,9 +360,9 @@ blkio_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes) WITH_QEMU_LOCK_GUARD(>blkio_lock) { blkioq_discard(s->blkioq, offset, bytes, , 0); -blkio_submit_io(bs); } +blkio_submit_io(bs); qemu_coroutine_yield(); return cod.ret; } @@ -378,9 +393,9 @@ blkio_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes, WITH_QEMU_LOCK_GUARD(>blkio_lock) { blkioq_readv(s->blkioq, offset, iov, iovcnt, , 0); -blkio_submit_io(bs); } +blkio_submit_io(bs); qemu_coroutine_yield(); if (use_bounce_buffer) { @@ -423,9 +438,9 @@ static int coroutine_fn blkio_co_pwritev(BlockDriverState *bs, int64_t offset, WITH_QEMU_LOCK_GUARD(>blkio_lock) { blkioq_writev(s->blkioq, offset, iov, iovcnt, , blkio_flags); -blkio_submit_io(bs); } +blkio_submit_io(bs); qemu_coroutine_yield(); if (use_bounce_buffer) { @@ -444,9 +459,9 @@ static int coroutine_fn blkio_co_flush(BlockDriverState *bs) WITH_QEMU_LOCK_GUARD(>blkio_lock) { blkioq_flush(s->blkioq, , 0); -blkio_submit_io(bs); } +blkio_submit_io(bs); qemu_coroutine_yield(); return cod.ret; } @@ -472,22 +487,13 @@ static int coroutine_fn blkio_co_pwrite_zeroes(BlockDriverState *bs, WITH_QEMU_LOCK_GUARD(>blkio_lock) { blkioq_write_zeroes(s->blkioq, offset, bytes, , blkio_flags); -blkio_submit_io(bs); } +blkio_submit_io(bs); qemu_coroutine_yield(); return cod.ret; } -static void coroutine_fn blkio_co_io_unplug(BlockDriverState *bs) -{ -BDRVBlkioState *s = bs->opaque; - -WITH_QEMU_LOCK_GUARD(>blkio_lock) { -blkio_submit_io(bs); -} -} - typedef enum { BMRR_OK, BMRR_SKIP, @@ -1009,7 +1015,6 @@ static void blkio_refresh_limits(BlockDriverState *bs, Error **errp) .bdrv_co_pwritev = blkio_co_pwritev, \ .bdrv_co_flush_to_disk = blkio_co_flush, \ .bdrv_co_pwrite_zeroes = blkio_co_pwrite_zeroes, \ -.bdrv_co_io_unplug = blkio_co_io_unplug, \ .bdrv_refresh_limits = blkio_refresh_limits, \ .bdrv_register_buf = blkio_register_buf, \ .bdrv_unregister_buf = blkio_unregister_buf, \ -- 2.40.1
[PATCH v2 1/6] block: add blk_io_plug_call() API
Introduce a new API for thread-local blk_io_plug() that does not traverse the block graph. The goal is to make blk_io_plug() multi-queue friendly. Instead of having block drivers track whether or not we're in a plugged section, provide an API that allows them to defer a function call until we're unplugged: blk_io_plug_call(fn, opaque). If blk_io_plug_call() is called multiple times with the same fn/opaque pair, then fn() is only called once at the end of the function - resulting in batching. This patch introduces the API and changes blk_io_plug()/blk_io_unplug(). blk_io_plug()/blk_io_unplug() no longer require a BlockBackend argument because the plug state is now thread-local. Later patches convert block drivers to blk_io_plug_call() and then we can finally remove .bdrv_co_io_plug() once all block drivers have been converted. Signed-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake --- v2 - "is not be freed" -> "is not freed" [Eric] --- MAINTAINERS | 1 + include/sysemu/block-backend-io.h | 13 +-- block/block-backend.c | 22 - block/plug.c | 159 ++ hw/block/dataplane/xen-block.c| 8 +- hw/block/virtio-blk.c | 4 +- hw/scsi/virtio-scsi.c | 6 +- block/meson.build | 1 + 8 files changed, 173 insertions(+), 41 deletions(-) create mode 100644 block/plug.c diff --git a/MAINTAINERS b/MAINTAINERS index 1b6466496d..2be6f0c26b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2646,6 +2646,7 @@ F: util/aio-*.c F: util/aio-*.h F: util/fdmon-*.c F: block/io.c +F: block/plug.c F: migration/block* F: include/block/aio.h F: include/block/aio-wait.h diff --git a/include/sysemu/block-backend-io.h b/include/sysemu/block-backend-io.h index d62a7ee773..be4dcef59d 100644 --- a/include/sysemu/block-backend-io.h +++ b/include/sysemu/block-backend-io.h @@ -100,16 +100,9 @@ void blk_iostatus_set_err(BlockBackend *blk, int error); int blk_get_max_iov(BlockBackend *blk); int blk_get_max_hw_iov(BlockBackend *blk); -/* - * blk_io_plug/unplug are thread-local operations. This means that multiple - * IOThreads can simultaneously call plug/unplug, but the caller must ensure - * that each unplug() is called in the same IOThread of the matching plug(). - */ -void coroutine_fn blk_co_io_plug(BlockBackend *blk); -void co_wrapper blk_io_plug(BlockBackend *blk); - -void coroutine_fn blk_co_io_unplug(BlockBackend *blk); -void co_wrapper blk_io_unplug(BlockBackend *blk); +void blk_io_plug(void); +void blk_io_unplug(void); +void blk_io_plug_call(void (*fn)(void *), void *opaque); AioContext *blk_get_aio_context(BlockBackend *blk); BlockAcctStats *blk_get_stats(BlockBackend *blk); diff --git a/block/block-backend.c b/block/block-backend.c index ca537cd0ad..1f1d226ba6 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -2568,28 +2568,6 @@ void blk_add_insert_bs_notifier(BlockBackend *blk, Notifier *notify) notifier_list_add(>insert_bs_notifiers, notify); } -void coroutine_fn blk_co_io_plug(BlockBackend *blk) -{ -BlockDriverState *bs = blk_bs(blk); -IO_CODE(); -GRAPH_RDLOCK_GUARD(); - -if (bs) { -bdrv_co_io_plug(bs); -} -} - -void coroutine_fn blk_co_io_unplug(BlockBackend *blk) -{ -BlockDriverState *bs = blk_bs(blk); -IO_CODE(); -GRAPH_RDLOCK_GUARD(); - -if (bs) { -bdrv_co_io_unplug(bs); -} -} - BlockAcctStats *blk_get_stats(BlockBackend *blk) { IO_CODE(); diff --git a/block/plug.c b/block/plug.c new file mode 100644 index 00..98a155d2f4 --- /dev/null +++ b/block/plug.c @@ -0,0 +1,159 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Block I/O plugging + * + * Copyright Red Hat. + * + * This API defers a function call within a blk_io_plug()/blk_io_unplug() + * section, allowing multiple calls to batch up. This is a performance + * optimization that is used in the block layer to submit several I/O requests + * at once instead of individually: + * + * blk_io_plug(); <-- start of plugged region + * ... + * blk_io_plug_call(my_func, my_obj); <-- deferred my_func(my_obj) call + * blk_io_plug_call(my_func, my_obj); <-- another + * blk_io_plug_call(my_func, my_obj); <-- another + * ... + * blk_io_unplug(); <-- end of plugged region, my_func(my_obj) is called once + * + * This code is actually generic and not tied to the block layer. If another + * subsystem needs this functionality, it could be renamed. + */ + +#include "qemu/osdep.h" +#include "qemu/coroutine-tls.h" +#include "qemu/notify.h" +#include "qemu/thread.h" +#include "sysemu/block-backend.h" + +/* A function call that has been deferred until unplug() */ +typedef struct { +void (*fn)(void *); +void *opaque; +} UnplugFn; + +/* Per-thread state */ +typedef struct { +unsigned count; /* how many times has plug() been called? */ +GArray *unplug_fns; /* functions to call at unplug time */ +} Plug; +
[PATCH v2 2/6] block/nvme: convert to blk_io_plug_call() API
Stop using the .bdrv_co_io_plug() API because it is not multi-queue block layer friendly. Use the new blk_io_plug_call() API to batch I/O submission instead. Signed-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake --- v2 - Remove unused nvme_process_completion_queue_plugged trace event [Stefano] --- block/nvme.c | 44 block/trace-events | 1 - 2 files changed, 12 insertions(+), 33 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index 5b744c2bda..100b38b592 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -25,6 +25,7 @@ #include "qemu/vfio-helpers.h" #include "block/block-io.h" #include "block/block_int.h" +#include "sysemu/block-backend.h" #include "sysemu/replay.h" #include "trace.h" @@ -119,7 +120,6 @@ struct BDRVNVMeState { int blkshift; uint64_t max_transfer; -bool plugged; bool supports_write_zeroes; bool supports_discard; @@ -282,7 +282,7 @@ static void nvme_kick(NVMeQueuePair *q) { BDRVNVMeState *s = q->s; -if (s->plugged || !q->need_kick) { +if (!q->need_kick) { return; } trace_nvme_kick(s, q->index); @@ -387,10 +387,6 @@ static bool nvme_process_completion(NVMeQueuePair *q) NvmeCqe *c; trace_nvme_process_completion(s, q->index, q->inflight); -if (s->plugged) { -trace_nvme_process_completion_queue_plugged(s, q->index); -return false; -} /* * Support re-entrancy when a request cb() function invokes aio_poll(). @@ -480,6 +476,15 @@ static void nvme_trace_command(const NvmeCmd *cmd) } } +static void nvme_unplug_fn(void *opaque) +{ +NVMeQueuePair *q = opaque; + +QEMU_LOCK_GUARD(>lock); +nvme_kick(q); +nvme_process_completion(q); +} + static void nvme_submit_command(NVMeQueuePair *q, NVMeRequest *req, NvmeCmd *cmd, BlockCompletionFunc cb, void *opaque) @@ -496,8 +501,7 @@ static void nvme_submit_command(NVMeQueuePair *q, NVMeRequest *req, q->sq.tail * NVME_SQ_ENTRY_BYTES, cmd, sizeof(*cmd)); q->sq.tail = (q->sq.tail + 1) % NVME_QUEUE_SIZE; q->need_kick++; -nvme_kick(q); -nvme_process_completion(q); +blk_io_plug_call(nvme_unplug_fn, q); qemu_mutex_unlock(>lock); } @@ -1567,27 +1571,6 @@ static void nvme_attach_aio_context(BlockDriverState *bs, } } -static void coroutine_fn nvme_co_io_plug(BlockDriverState *bs) -{ -BDRVNVMeState *s = bs->opaque; -assert(!s->plugged); -s->plugged = true; -} - -static void coroutine_fn nvme_co_io_unplug(BlockDriverState *bs) -{ -BDRVNVMeState *s = bs->opaque; -assert(s->plugged); -s->plugged = false; -for (unsigned i = INDEX_IO(0); i < s->queue_count; i++) { -NVMeQueuePair *q = s->queues[i]; -qemu_mutex_lock(>lock); -nvme_kick(q); -nvme_process_completion(q); -qemu_mutex_unlock(>lock); -} -} - static bool nvme_register_buf(BlockDriverState *bs, void *host, size_t size, Error **errp) { @@ -1664,9 +1647,6 @@ static BlockDriver bdrv_nvme = { .bdrv_detach_aio_context = nvme_detach_aio_context, .bdrv_attach_aio_context = nvme_attach_aio_context, -.bdrv_co_io_plug = nvme_co_io_plug, -.bdrv_co_io_unplug= nvme_co_io_unplug, - .bdrv_register_buf= nvme_register_buf, .bdrv_unregister_buf = nvme_unregister_buf, }; diff --git a/block/trace-events b/block/trace-events index 32665158d6..048ad27519 100644 --- a/block/trace-events +++ b/block/trace-events @@ -141,7 +141,6 @@ nvme_kick(void *s, unsigned q_index) "s %p q #%u" nvme_dma_flush_queue_wait(void *s) "s %p" nvme_error(int cmd_specific, int sq_head, int sqid, int cid, int status) "cmd_specific %d sq_head %d sqid %d cid %d status 0x%x" nvme_process_completion(void *s, unsigned q_index, int inflight) "s %p q #%u inflight %d" -nvme_process_completion_queue_plugged(void *s, unsigned q_index) "s %p q #%u" nvme_complete_command(void *s, unsigned q_index, int cid) "s %p q #%u cid %d" nvme_submit_command(void *s, unsigned q_index, int cid) "s %p q #%u cid %d" nvme_submit_command_raw(int c0, int c1, int c2, int c3, int c4, int c5, int c6, int c7) "%02x %02x %02x %02x %02x %02x %02x %02x" -- 2.40.1
[PATCH v2 4/6] block/io_uring: convert to blk_io_plug_call() API
Stop using the .bdrv_co_io_plug() API because it is not multi-queue block layer friendly. Use the new blk_io_plug_call() API to batch I/O submission instead. Signed-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake --- v2 - Removed whitespace hunk [Eric] --- include/block/raw-aio.h | 7 --- block/file-posix.c | 10 -- block/io_uring.c| 44 - block/trace-events | 5 ++--- 4 files changed, 19 insertions(+), 47 deletions(-) diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h index 0fe85ade77..da60ca13ef 100644 --- a/include/block/raw-aio.h +++ b/include/block/raw-aio.h @@ -81,13 +81,6 @@ int coroutine_fn luring_co_submit(BlockDriverState *bs, int fd, uint64_t offset, QEMUIOVector *qiov, int type); void luring_detach_aio_context(LuringState *s, AioContext *old_context); void luring_attach_aio_context(LuringState *s, AioContext *new_context); - -/* - * luring_io_plug/unplug work in the thread's current AioContext, therefore the - * caller must ensure that they are paired in the same IOThread. - */ -void luring_io_plug(void); -void luring_io_unplug(void); #endif #ifdef _WIN32 diff --git a/block/file-posix.c b/block/file-posix.c index 0ab158efba..7baa8491dd 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -2558,11 +2558,6 @@ static void coroutine_fn raw_co_io_plug(BlockDriverState *bs) laio_io_plug(); } #endif -#ifdef CONFIG_LINUX_IO_URING -if (s->use_linux_io_uring) { -luring_io_plug(); -} -#endif } static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs) @@ -2573,11 +2568,6 @@ static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs) laio_io_unplug(s->aio_max_batch); } #endif -#ifdef CONFIG_LINUX_IO_URING -if (s->use_linux_io_uring) { -luring_io_unplug(); -} -#endif } static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs) diff --git a/block/io_uring.c b/block/io_uring.c index 82cab6a5bd..4e25b56c9c 100644 --- a/block/io_uring.c +++ b/block/io_uring.c @@ -16,6 +16,7 @@ #include "block/raw-aio.h" #include "qemu/coroutine.h" #include "qapi/error.h" +#include "sysemu/block-backend.h" #include "trace.h" /* Only used for assertions. */ @@ -41,7 +42,6 @@ typedef struct LuringAIOCB { } LuringAIOCB; typedef struct LuringQueue { -int plugged; unsigned int in_queue; unsigned int in_flight; bool blocked; @@ -267,7 +267,7 @@ static void luring_process_completions_and_submit(LuringState *s) { luring_process_completions(s); -if (!s->io_q.plugged && s->io_q.in_queue > 0) { +if (s->io_q.in_queue > 0) { ioq_submit(s); } } @@ -301,29 +301,17 @@ static void qemu_luring_poll_ready(void *opaque) static void ioq_init(LuringQueue *io_q) { QSIMPLEQ_INIT(_q->submit_queue); -io_q->plugged = 0; io_q->in_queue = 0; io_q->in_flight = 0; io_q->blocked = false; } -void luring_io_plug(void) +static void luring_unplug_fn(void *opaque) { -AioContext *ctx = qemu_get_current_aio_context(); -LuringState *s = aio_get_linux_io_uring(ctx); -trace_luring_io_plug(s); -s->io_q.plugged++; -} - -void luring_io_unplug(void) -{ -AioContext *ctx = qemu_get_current_aio_context(); -LuringState *s = aio_get_linux_io_uring(ctx); -assert(s->io_q.plugged); -trace_luring_io_unplug(s, s->io_q.blocked, s->io_q.plugged, - s->io_q.in_queue, s->io_q.in_flight); -if (--s->io_q.plugged == 0 && -!s->io_q.blocked && s->io_q.in_queue > 0) { +LuringState *s = opaque; +trace_luring_unplug_fn(s, s->io_q.blocked, s->io_q.in_queue, + s->io_q.in_flight); +if (!s->io_q.blocked && s->io_q.in_queue > 0) { ioq_submit(s); } } @@ -370,14 +358,16 @@ static int luring_do_submit(int fd, LuringAIOCB *luringcb, LuringState *s, QSIMPLEQ_INSERT_TAIL(>io_q.submit_queue, luringcb, next); s->io_q.in_queue++; -trace_luring_do_submit(s, s->io_q.blocked, s->io_q.plugged, - s->io_q.in_queue, s->io_q.in_flight); -if (!s->io_q.blocked && -(!s->io_q.plugged || - s->io_q.in_flight + s->io_q.in_queue >= MAX_ENTRIES)) { -ret = ioq_submit(s); -trace_luring_do_submit_done(s, ret); -return ret; +trace_luring_do_submit(s, s->io_q.blocked, s->io_q.in_queue, + s->io_q.in_flight); +if (!s->io_q.blocked) { +if (s->io_q.in_flight + s->io_q.in_queue >= MAX_ENTRIES) { +ret = ioq_submit(s); +trace_luring_do_submit_done(s, ret); +return ret; +} + +blk_io_plug_call(luring_unplug_fn, s); } return 0; } diff --git a/block/trace-events b/block/trace-events index 048ad27519..6f121b7636 100644 --- a/block/trace-events +++ b/block/trace-events @@ -64,9 +64,8 @@ file_paio_submit(void *acb, void
[PATCH v2 0/6] block: add blk_io_plug_call() API
v2 - Patch 1: "is not be freed" -> "is not freed" [Eric] - Patch 2: Remove unused nvme_process_completion_queue_plugged trace event [Stefano] - Patch 3: Add missing #include and fix blkio_unplug_fn() prototype [Stefano] - Patch 4: Removed whitespace hunk [Eric] The existing blk_io_plug() API is not block layer multi-queue friendly because the plug state is per-BlockDriverState. Change blk_io_plug()'s implementation so it is thread-local. This is done by introducing the blk_io_plug_call() function that block drivers use to batch calls while plugged. It is relatively easy to convert block drivers from .bdrv_co_io_plug() to blk_io_plug_call(). Random read 4KB performance with virtio-blk on a host NVMe block device: iodepth iops change vs today 145612 -4% 287967 +2% 4 129872 +0% 8 171096 -3% 16 194508 -4% 32 208947 -1% 64 217647 +0% 128 229629 +0% The results are within the noise for these benchmarks. This is to be expected because the plugging behavior for a single thread hasn't changed in this patch series, only that the state is thread-local now. The following graph compares several approaches: https://vmsplice.net/~stefan/blk_io_plug-thread-local.png - v7.2.0: before most of the multi-queue block layer changes landed. - with-blk_io_plug: today's post-8.0.0 QEMU. - blk_io_plug-thread-local: this patch series. - no-blk_io_plug: what happens when we simply remove plugging? - call-after-dispatch: what if we integrate plugging into the event loop? I decided against this approach in the end because it's more likely to introduce performance regressions since I/O submission is deferred until the end of the event loop iteration. Aside from the no-blk_io_plug case, which bottlenecks much earlier than the others, we see that all plugging approaches are more or less equivalent in this benchmark. It is also clear that QEMU 8.0.0 has lower performance than 7.2.0. The Ansible playbook, fio results, and a Jupyter notebook are available here: https://github.com/stefanha/qemu-perf/tree/remove-blk_io_plug Stefan Hajnoczi (6): block: add blk_io_plug_call() API block/nvme: convert to blk_io_plug_call() API block/blkio: convert to blk_io_plug_call() API block/io_uring: convert to blk_io_plug_call() API block/linux-aio: convert to blk_io_plug_call() API block: remove bdrv_co_io_plug() API MAINTAINERS | 1 + include/block/block-io.h | 3 - include/block/block_int-common.h | 11 --- include/block/raw-aio.h | 14 --- include/sysemu/block-backend-io.h | 13 +-- block/blkio.c | 43 block/block-backend.c | 22 - block/file-posix.c| 38 --- block/io.c| 37 --- block/io_uring.c | 44 - block/linux-aio.c | 41 +++- block/nvme.c | 44 +++-- block/plug.c | 159 ++ hw/block/dataplane/xen-block.c| 8 +- hw/block/virtio-blk.c | 4 +- hw/scsi/virtio-scsi.c | 6 +- block/meson.build | 1 + block/trace-events| 6 +- 18 files changed, 239 insertions(+), 256 deletions(-) create mode 100644 block/plug.c -- 2.40.1
Re: [PATCH v7 10/12] xen/tools: add sve parameter in XL configuration
On Tue, May 23, 2023 at 08:43:24AM +0100, Luca Fancellu wrote: > Add sve parameter in XL configuration to allow guests to use > SVE feature. > > Signed-off-by: Luca Fancellu Reviewed-by: Anthony PERARD Thanks, -- Anthony PERARD
[XEN PATCH 10/15] build: rename $(AFLAGS) to $(XEN_AFLAGS)
We don't want the AFLAGS from the environment, they are usually meant to build user space application and not for the hypervisor. Config.mk doesn't provied any $(AFLAGS) so we can start a fresh $(XEN_AFLAGS). Signed-off-by: Anthony PERARD --- xen/Makefile | 10 ++ xen/arch/x86/arch.mk | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/xen/Makefile b/xen/Makefile index 127c0e40b5..c4a83fca76 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -258,6 +258,8 @@ export KBUILD_DEFCONFIG := $(ARCH)_defconfig # reparsing Config.mk by e.g. arch/x86/boot/. export XEN_TREEWIDE_CFLAGS := $(CFLAGS) +XEN_AFLAGS = + # CLANG_FLAGS needs to be calculated before calling Kconfig ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),) CLANG_FLAGS := @@ -412,9 +414,9 @@ ifneq ($(CONFIG_CC_IS_CLANG),y) CFLAGS += -Wa,--strip-local-absolute endif -AFLAGS += -D__ASSEMBLY__ +XEN_AFLAGS += -D__ASSEMBLY__ -$(call cc-option-add,AFLAGS,CC,-Wa$(comma)--noexecstack) +$(call cc-option-add,XEN_AFLAGS,CC,-Wa$(comma)--noexecstack) LDFLAGS-$(call ld-option,--warn-rwx-segments) += --no-warn-rwx-segments @@ -425,7 +427,7 @@ CFLAGS += $(EXTRA_CFLAGS_XEN_CORE) # Most CFLAGS are safe for assembly files: # -std=gnu{89,99} gets confused by #-prefixed end-of-line comments # -flto makes no sense and annoys clang -AFLAGS += $(filter-out -std=gnu% -flto,$(CFLAGS)) $(AFLAGS-y) +XEN_AFLAGS += $(filter-out -std=gnu% -flto,$(CFLAGS)) $(AFLAGS-y) # LDFLAGS are only passed directly to $(LD) LDFLAGS += $(LDFLAGS_DIRECT) $(LDFLAGS-y) @@ -462,7 +464,7 @@ include $(srctree)/arch/$(TARGET_ARCH)/arch.mk # define new variables to avoid the ones defined in Config.mk export XEN_CFLAGS := $(CFLAGS) -export XEN_AFLAGS := $(AFLAGS) +export XEN_AFLAGS := $(XEN_AFLAGS) export XEN_LDFLAGS := $(LDFLAGS) export CFLAGS_UBSAN diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk index 7b5be9fe46..13ec88a628 100644 --- a/xen/arch/x86/arch.mk +++ b/xen/arch/x86/arch.mk @@ -80,7 +80,7 @@ ifeq ($(CONFIG_LD_IS_GNU),y) AFLAGS-$(call ld-option,--print-output-format) += -DHAVE_LD_SORT_BY_INIT_PRIORITY else # Assume all versions of LLD support this. -AFLAGS += -DHAVE_LD_SORT_BY_INIT_PRIORITY +XEN_AFLAGS += -DHAVE_LD_SORT_BY_INIT_PRIORITY endif ifneq ($(CONFIG_PV_SHIM_EXCLUSIVE),y) -- Anthony PERARD
[XEN PATCH 12/15] build: avoid Config.mk's CFLAGS
The variable $(CFLAGS) is too often set in the environment, especially when building a package for a distribution. Often, those CFLAGS are intended to be use to build user spaces binaries, not a kernel. This mean packager needs to takes extra steps to build Xen by overriding the CFLAGS provided by the package build environment. With this patch, we avoid using the variable $(CFLAGS). Also, the hypervisor's build system have complete control over which CFLAGS are used. No change intended to XEN_CFLAGS used, beside some flags which may be in a different order on the command line. Signed-off-by: Anthony PERARD --- Notes: There's still $(EXTRA_CFLAGS_XEN_CORE) which allows to add more CFLAGS if someone building Xen needs to add more CFLAGS to the hypervisor build. xen/Makefile | 11 ++- xen/arch/arm/arch.mk | 4 xen/arch/x86/arch.mk | 2 ++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/xen/Makefile b/xen/Makefile index b3bffe8c6f..4dc3acf2a6 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -259,7 +259,16 @@ export KBUILD_DEFCONFIG := $(ARCH)_defconfig export XEN_TREEWIDE_CFLAGS := $(CFLAGS) XEN_AFLAGS = -XEN_CFLAGS = $(CFLAGS) +XEN_CFLAGS = +ifeq ($(XEN_OS),SunOS) +XEN_CFLAGS += -Wa,--divide -D_POSIX_C_SOURCE=200112L -D__EXTENSIONS__ +endif +XEN_CFLAGS += -fno-strict-aliasing +XEN_CFLAGS += -std=gnu99 +XEN_CFLAGS += -Wall -Wstrict-prototypes +$(call cc-option-add,XEN_CFLAGS,CC,-Wdeclaration-after-statement) +$(call cc-option-add,XEN_CFLAGS,CC,-Wno-unused-but-set-variable) +$(call cc-option-add,XEN_CFLAGS,CC,-Wno-unused-local-typedefs) # CLANG_FLAGS needs to be calculated before calling Kconfig ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),) diff --git a/xen/arch/arm/arch.mk b/xen/arch/arm/arch.mk index 300b8bf7ae..0478fadde2 100644 --- a/xen/arch/arm/arch.mk +++ b/xen/arch/arm/arch.mk @@ -1,6 +1,10 @@ # arm-specific definitions +ifeq ($(XEN_TARGET_ARCH),arm32) +XEN_CFLAGS += -marm +endif + $(call cc-options-add,XEN_CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS)) $(call cc-option-add,XEN_CFLAGS,CC,-Wnested-externs) diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk index 5df3cf6bc3..fc3b1dc922 100644 --- a/xen/arch/x86/arch.mk +++ b/xen/arch/x86/arch.mk @@ -1,6 +1,8 @@ # x86-specific definitions +XEN_CFLAGS += -m64 + export XEN_IMG_OFFSET := 0x20 XEN_CFLAGS += -I$(srctree)/arch/x86/include/asm/mach-generic -- Anthony PERARD
[XEN PATCH 15/15] build: remove Config.mk include from Rules.mk
Everything needed to build the hypervisor should already be configured by "xen/Makefile", thus Config.mk shouldn't be needed. Then, Config.mk keeps on testing support of some CFLAGS with CC, the result of this testing is not used at this stage so the build is slowed unnecessarily. Likewise, GCC is checked to be at the minimum at 4.2 when entering every sub-directory, so the check have run countless time at this stage. We only need to export a few more configuration variables. And add some variables in Kbuild.include, and macro fallbacks for Make older than 3.81. (Adding `or` just in case. it's only used in xen/Makefile, which includes Config.mk and so has already the fallback.) Signed-off-by: Anthony PERARD --- Notes: Removing Config.mk benefit: Rebuild on my computer is aboud 1 second faster overall. Save maybe 3 seconds of user time system less loaded xen/Makefile | 4 xen/Rules.mk | 1 - xen/scripts/Kbuild.include | 7 +++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/xen/Makefile b/xen/Makefile index 4dc3acf2a6..9af7223c66 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -246,10 +246,14 @@ export TARGET_ARCH := $(shell echo $(XEN_TARGET_ARCH) | \ sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' \ -e s'/riscv.*/riscv/g') +export XEN_COMPILE_ARCH XEN_TARGET_ARCH export CONFIG_SHELL := $(SHELL) export CC CXX LD NM OBJCOPY OBJDUMP ADDR2LINE +export CPP AR export YACC = $(if $(BISON),$(BISON),bison) export LEX = $(if $(FLEX),$(FLEX),flex) +export HOSTCC HOSTCXX HOSTCFLAGS +export EMBEDDED_EXTRA_CFLAGS LDFLAGS_DIRECT # Default file for 'make defconfig'. export KBUILD_DEFCONFIG := $(ARCH)_defconfig diff --git a/xen/Rules.mk b/xen/Rules.mk index 8177d405c3..8291e0a573 100644 --- a/xen/Rules.mk +++ b/xen/Rules.mk @@ -17,7 +17,6 @@ __build: -include $(objtree)/include/config/auto.conf -include $(XEN_ROOT)/Config.mk include $(srctree)/scripts/Kbuild.include include $(XEN_ROOT)/config/compiler-testing.mk diff --git a/xen/scripts/Kbuild.include b/xen/scripts/Kbuild.include index d820595e2f..dfa66f2c8a 100644 --- a/xen/scripts/Kbuild.include +++ b/xen/scripts/Kbuild.include @@ -8,6 +8,13 @@ empty := space := $(empty) $(empty) space_escape := _-_SPACE_-_ pound := \# +comma := , +open:= ( +close := ) + +# fallbacks for GNU Make older than 3.81 +realpath = $(wildcard $(foreach file,$(1),$(shell cd -P $(dir $(file)) && echo "$$PWD/$(notdir $(file))"))) +or = $(if $(strip $(1)),$(1),$(if $(strip $(2)),$(2),$(if $(strip $(3)),$(3),$(if $(strip $(4)),$(4) ### # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o -- Anthony PERARD
Re: [XEN PATCH] tools/xendomains: Don't auto save/restore/migrate on Arm*
On Mon, May 22, 2023 at 08:34:52PM +0100, Julien Grall wrote: > On 19/05/2023 17:24, Anthony PERARD wrote: > > diff --git a/tools/configure.ac b/tools/configure.ac > > index 3cccf41960..0f0983f6b7 100644 > > --- a/tools/configure.ac > > +++ b/tools/configure.ac > > @@ -517,4 +517,17 @@ AS_IF([test "x$pvshim" = "xy"], [ > > AX_FIND_HEADER([INCLUDE_ENDIAN_H], [endian.h sys/endian.h]) > > +dnl Disable autosave of domain in xendomains on shutdown > > +dnl due to missing support. This should be in sync with > > +dnl LIBXL_HAVE_NO_SUSPEND_RESUME in libxl.h > > Quite likely, a developer adding a new arch will first look at the > definition of LIXBL_HAVE_NO_SUSPEND_RESUME. So it would be good if we have a > similar message there to remind them to update this case. That said... Probably true, I'll look at adding a comment there. > > +case "$host_cpu" in > > +arm*|aarch64) > > +XENDOMAINS_SAVE_DIR= > > +;; > > +*) > > +XENDOMAINS_SAVE_DIR="$XEN_LIB_DIR/save" > > +;; > > +esac > > ... I am wondering if the switch should be the other way around. IOW, the > default should be no support for suspend/resume. This will make easier to > add support for RISC-V (I suspect support for suspend/resume will not be in > the first version) or any new other arch. Sounds good, I'll look at that. Thanks, -- Anthony PERARD
[XEN PATCH 14/15] Config.mk: move $(cc-option, ) to config/compiler-testing.mk
In xen/, it isn't necessary to include Config.mk in every Makefile in subdirectories as nearly all necessary variables should be calculated in xen/Makefile. But some Makefile make use of the macro $(cc-option,) that is only available in Config.mk. Extract $(cc-option,) from Config.mk so we can use it without including Config.mk and thus without having to recalculate some CFLAGS which would be ignored. Signed-off-by: Anthony PERARD --- Config.mk | 27 +-- config/compiler-testing.mk | 25 + xen/Rules.mk | 1 + 3 files changed, 27 insertions(+), 26 deletions(-) create mode 100644 config/compiler-testing.mk diff --git a/Config.mk b/Config.mk index 27f48f654a..ebc8d0dfdd 100644 --- a/Config.mk +++ b/Config.mk @@ -18,6 +18,7 @@ realpath = $(wildcard $(foreach file,$(1),$(shell cd -P $(dir $(file)) && echo " or = $(if $(strip $(1)),$(1),$(if $(strip $(2)),$(2),$(if $(strip $(3)),$(3),$(if $(strip $(4)),$(4) -include $(XEN_ROOT)/.config +include $(XEN_ROOT)/config/compiler-testing.mk XEN_COMPILE_ARCH?= $(shell uname -m | sed -e s/i.86/x86_32/ \ -e s/i86pc/x86_32/ -e s/amd64/x86_64/ \ @@ -79,32 +80,6 @@ PYTHON_PREFIX_ARG ?= --prefix="$(prefix)" # to permit the user to set PYTHON_PREFIX_ARG to '' to workaround this bug: # https://bugs.launchpad.net/ubuntu/+bug/362570 -# cc-option: Check if compiler supports first option, else fall back to second. -# -# This is complicated by the fact that unrecognised -Wno-* options: -# (a) are ignored unless the compilation emits a warning; and -# (b) even then produce a warning rather than an error -# To handle this we do a test compile, passing the option-under-test, on a code -# fragment that will always produce a warning (integer assigned to pointer). -# We then grep for the option-under-test in the compiler's output, the presence -# of which would indicate an "unrecognized command-line option" warning/error. -# -# Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586) -cc-option = $(shell if test -z "`echo 'void*p=1;' | \ - $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- $(2:-Wa$(comma)%=%) -`"; \ - then echo "$(2)"; else echo "$(3)"; fi ;) - -# cc-option-add: Add an option to compilation flags, but only if supported. -# Usage: $(call cc-option-add CFLAGS,CC,-march=winchip-c6) -cc-option-add = $(eval $(call cc-option-add-closure,$(1),$(2),$(3))) -define cc-option-add-closure -ifneq ($$(call cc-option,$$($(2)),$(3),n),n) -$(1) += $(3) -endif -endef - -cc-options-add = $(foreach o,$(3),$(call cc-option-add,$(1),$(2),$(o))) - # cc-ver: Check compiler against the version requirement. Return boolean 'y'/'n'. # Usage: ifeq ($(call cc-ver,$(CC),ge,0x030400),y) cc-ver = $(shell if [ $$((`$(1) -dumpversion | awk -F. \ diff --git a/config/compiler-testing.mk b/config/compiler-testing.mk new file mode 100644 index 00..9677f91abe --- /dev/null +++ b/config/compiler-testing.mk @@ -0,0 +1,25 @@ +# cc-option: Check if compiler supports first option, else fall back to second. +# +# This is complicated by the fact that unrecognised -Wno-* options: +# (a) are ignored unless the compilation emits a warning; and +# (b) even then produce a warning rather than an error +# To handle this we do a test compile, passing the option-under-test, on a code +# fragment that will always produce a warning (integer assigned to pointer). +# We then grep for the option-under-test in the compiler's output, the presence +# of which would indicate an "unrecognized command-line option" warning/error. +# +# Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586) +cc-option = $(shell if test -z "`echo 'void*p=1;' | \ + $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- $(2:-Wa$(comma)%=%) -`"; \ + then echo "$(2)"; else echo "$(3)"; fi ;) + +# cc-option-add: Add an option to compilation flags, but only if supported. +# Usage: $(call cc-option-add CFLAGS,CC,-march=winchip-c6) +cc-option-add = $(eval $(call cc-option-add-closure,$(1),$(2),$(3))) +define cc-option-add-closure +ifneq ($$(call cc-option,$$($(2)),$(3),n),n) +$(1) += $(3) +endif +endef + +cc-options-add = $(foreach o,$(3),$(call cc-option-add,$(1),$(2),$(o))) diff --git a/xen/Rules.mk b/xen/Rules.mk index 68b10ca5ef..8177d405c3 100644 --- a/xen/Rules.mk +++ b/xen/Rules.mk @@ -19,6 +19,7 @@ __build: include $(XEN_ROOT)/Config.mk include $(srctree)/scripts/Kbuild.include +include $(XEN_ROOT)/config/compiler-testing.mk # Initialise some variables obj-y := -- Anthony PERARD
[XEN PATCH 11/15] build: rename CFLAGS to XEN_CFLAGS in xen/Makefile
This is a preparatory patch. A future patch will not even use $(CFLAGS) to seed $(XEN_CFLAGS). Signed-off-by: Anthony PERARD --- xen/Makefile | 41 ++--- xen/arch/arm/arch.mk | 4 +-- xen/arch/riscv/arch.mk | 4 +-- xen/arch/x86/arch.mk | 58 +- 4 files changed, 54 insertions(+), 53 deletions(-) diff --git a/xen/Makefile b/xen/Makefile index c4a83fca76..b3bffe8c6f 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -259,6 +259,7 @@ export KBUILD_DEFCONFIG := $(ARCH)_defconfig export XEN_TREEWIDE_CFLAGS := $(CFLAGS) XEN_AFLAGS = +XEN_CFLAGS = $(CFLAGS) # CLANG_FLAGS needs to be calculated before calling Kconfig ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),) @@ -284,7 +285,7 @@ CLANG_FLAGS += $(call or,$(t1),$(t2),$(t3)) endif CLANG_FLAGS += -Werror=unknown-warning-option -CFLAGS += $(CLANG_FLAGS) +XEN_CFLAGS += $(CLANG_FLAGS) export CLANG_FLAGS endif @@ -293,7 +294,7 @@ ifeq ($(call ld-ver-build-id,$(LD)),n) XEN_LDFLAGS_BUILD_ID := XEN_HAS_BUILD_ID := n else -CFLAGS += -DBUILD_ID +XEN_CFLAGS += -DBUILD_ID XEN_TREEWIDE_CFLAGS += -DBUILD_ID XEN_HAS_BUILD_ID := y XEN_LDFLAGS_BUILD_ID := --build-id=sha1 @@ -388,30 +389,30 @@ include/config/%.conf include/config/%.conf.cmd: $(KCONFIG_CONFIG) $(Q)$(MAKE) $(build)=tools/kconfig syncconfig ifeq ($(CONFIG_DEBUG),y) -CFLAGS += -O1 +XEN_CFLAGS += -O1 else -CFLAGS += -O2 +XEN_CFLAGS += -O2 endif ifeq ($(CONFIG_FRAME_POINTER),y) -CFLAGS += -fno-omit-frame-pointer +XEN_CFLAGS += -fno-omit-frame-pointer else -CFLAGS += -fomit-frame-pointer +XEN_CFLAGS += -fomit-frame-pointer endif CFLAGS-$(CONFIG_CC_SPLIT_SECTIONS) += -ffunction-sections -fdata-sections -CFLAGS += -nostdinc -fno-builtin -fno-common -CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith -$(call cc-option-add,CFLAGS,CC,-Wvla) -CFLAGS += -pipe -D__XEN__ -include $(srctree)/include/xen/config.h +XEN_CFLAGS += -nostdinc -fno-builtin -fno-common +XEN_CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith +$(call cc-option-add,XEN_CFLAGS,CC,-Wvla) +XEN_CFLAGS += -pipe -D__XEN__ -include $(srctree)/include/xen/config.h CFLAGS-$(CONFIG_DEBUG_INFO) += -g ifneq ($(CONFIG_CC_IS_CLANG),y) # Clang doesn't understand this command line argument, and doesn't appear to # have a suitable alternative. The resulting compiled binary does function, # but has an excessively large symbol table. -CFLAGS += -Wa,--strip-local-absolute +XEN_CFLAGS += -Wa,--strip-local-absolute endif XEN_AFLAGS += -D__ASSEMBLY__ @@ -420,14 +421,14 @@ $(call cc-option-add,XEN_AFLAGS,CC,-Wa$(comma)--noexecstack) LDFLAGS-$(call ld-option,--warn-rwx-segments) += --no-warn-rwx-segments -CFLAGS += $(CFLAGS-y) +XEN_CFLAGS += $(CFLAGS-y) # allow extra CFLAGS externally via EXTRA_CFLAGS_XEN_CORE -CFLAGS += $(EXTRA_CFLAGS_XEN_CORE) +XEN_CFLAGS += $(EXTRA_CFLAGS_XEN_CORE) # Most CFLAGS are safe for assembly files: # -std=gnu{89,99} gets confused by #-prefixed end-of-line comments # -flto makes no sense and annoys clang -XEN_AFLAGS += $(filter-out -std=gnu% -flto,$(CFLAGS)) $(AFLAGS-y) +XEN_AFLAGS += $(filter-out -std=gnu% -flto,$(XEN_CFLAGS)) $(AFLAGS-y) # LDFLAGS are only passed directly to $(LD) LDFLAGS += $(LDFLAGS_DIRECT) $(LDFLAGS-y) @@ -439,16 +440,16 @@ CFLAGS_UBSAN := endif ifeq ($(CONFIG_LTO),y) -CFLAGS += -flto +XEN_CFLAGS += -flto LDFLAGS-$(CONFIG_CC_IS_CLANG) += -plugin LLVMgold.so endif ifdef building_out_of_srctree -CFLAGS += -I$(objtree)/include -CFLAGS += -I$(objtree)/arch/$(TARGET_ARCH)/include +XEN_CFLAGS += -I$(objtree)/include +XEN_CFLAGS += -I$(objtree)/arch/$(TARGET_ARCH)/include endif -CFLAGS += -I$(srctree)/include -CFLAGS += -I$(srctree)/arch/$(TARGET_ARCH)/include +XEN_CFLAGS += -I$(srctree)/include +XEN_CFLAGS += -I$(srctree)/arch/$(TARGET_ARCH)/include # Note that link order matters! ALL_OBJS-y:= common/built_in.o @@ -463,7 +464,7 @@ ALL_LIBS-y:= lib/lib.a include $(srctree)/arch/$(TARGET_ARCH)/arch.mk # define new variables to avoid the ones defined in Config.mk -export XEN_CFLAGS := $(CFLAGS) +export XEN_CFLAGS := $(XEN_CFLAGS) export XEN_AFLAGS := $(XEN_AFLAGS) export XEN_LDFLAGS := $(LDFLAGS) export CFLAGS_UBSAN diff --git a/xen/arch/arm/arch.mk b/xen/arch/arm/arch.mk index 58db76c4e1..300b8bf7ae 100644 --- a/xen/arch/arm/arch.mk +++ b/xen/arch/arm/arch.mk @@ -1,8 +1,8 @@ # arm-specific definitions -$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS)) -$(call cc-option-add,CFLAGS,CC,-Wnested-externs) +$(call cc-options-add,XEN_CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS)) +$(call cc-option-add,XEN_CFLAGS,CC,-Wnested-externs) # Prevent floating-point variables from creeping into Xen. CFLAGS-$(CONFIG_ARM_32) += -msoft-float diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk index 7448f759b4..aadf373ce8 100644 --- a/xen/arch/riscv/arch.mk
[XEN PATCH 05/15] build: introduce a generic command for gzip
Make the gzip command generic and use -9 which wasn't use for config.gz. (xen.gz does use -9) Signed-off-by: Anthony PERARD --- xen/Rules.mk| 5 + xen/common/Makefile | 8 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/xen/Rules.mk b/xen/Rules.mk index 59072ae8df..68b10ca5ef 100644 --- a/xen/Rules.mk +++ b/xen/Rules.mk @@ -63,6 +63,11 @@ cmd_objcopy = $(OBJCOPY) $(OBJCOPYFLAGS) $< $@ quiet_cmd_binfile = BINFILE $@ cmd_binfile = $(SHELL) $(srctree)/tools/binfile $(BINFILE_FLAGS) $@ $(2) +# gzip +quiet_cmd_gzip = GZIP$@ +cmd_gzip = \ +cat $(real-prereqs) | gzip -n -f -9 > $@ + # Figure out what we need to build from the various variables # === diff --git a/xen/common/Makefile b/xen/common/Makefile index 46049eac35..f45f19c391 100644 --- a/xen/common/Makefile +++ b/xen/common/Makefile @@ -78,13 +78,13 @@ obj-$(CONFIG_NEEDS_LIBELF) += libelf/ obj-$(CONFIG_HAS_DEVICE_TREE) += libfdt/ CONF_FILE := $(if $(patsubst /%,,$(KCONFIG_CONFIG)),$(objtree)/)$(KCONFIG_CONFIG) -$(obj)/config.gz: $(CONF_FILE) - gzip -n -c $< >$@ +$(obj)/config.gz: $(CONF_FILE) FORCE + $(call if_changed,gzip) + +targets += config.gz $(obj)/config_data.o: $(obj)/config.gz $(obj)/config_data.S: $(srctree)/tools/binfile FORCE $(call if_changed,binfile,$(obj)/config.gz xen_config_data) targets += config_data.S - -clean-files := config.gz -- Anthony PERARD
[XEN PATCH 13/15] build: fix compile.h compiler version command line
CFLAGS is just from Config.mk, instead use the flags used to build Xen. Signed-off-by: Anthony PERARD --- Notes: I don't know if CFLAGS is even useful there, just --version without the flags might produce the same result. xen/build.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/build.mk b/xen/build.mk index e2a78aa806..d468bb6e26 100644 --- a/xen/build.mk +++ b/xen/build.mk @@ -23,7 +23,7 @@ define cmd_compile.h -e 's/@@whoami@@/$(XEN_WHOAMI)/g' \ -e 's/@@domain@@/$(XEN_DOMAIN)/g' \ -e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \ - -e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) --version 2>&1 | head -1)!g' \ + -e 's!@@compiler@@!$(shell $(CC) $(XEN_CFLAGS) --version 2>&1 | head -1)!g' \ -e 's/@@version@@/$(XEN_VERSION)/g' \ -e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \ -e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \ -- Anthony PERARD
[XEN PATCH 09/15] build: hide commands run for kconfig
but still show a log entry for syncconfig. We have to use kecho instead of $(cmd,) to avoid issue with prompt from kconfig. linux commits for reference: 23cd88c91343 ("kbuild: hide commands to run Kconfig, and show short log for syncconfig") d952cfaf0cff ("kbuild: do not suppress Kconfig prompts for silent build") Signed-off-by: Anthony PERARD --- xen/Makefile | 1 + xen/tools/kconfig/Makefile | 14 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/xen/Makefile b/xen/Makefile index 4dc960df2c..127c0e40b5 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -382,6 +382,7 @@ $(KCONFIG_CONFIG): tools_fixdep # This exploits the 'multi-target pattern rule' trick. # The syncconfig should be executed only once to make all the targets. include/config/%.conf include/config/%.conf.cmd: $(KCONFIG_CONFIG) + $(Q)$(kecho) " SYNC$@" $(Q)$(MAKE) $(build)=tools/kconfig syncconfig ifeq ($(CONFIG_DEBUG),y) diff --git a/xen/tools/kconfig/Makefile b/xen/tools/kconfig/Makefile index b7b9a419ad..18c0f5d4f1 100644 --- a/xen/tools/kconfig/Makefile +++ b/xen/tools/kconfig/Makefile @@ -24,19 +24,19 @@ endif unexport CONFIG_ xconfig: $(obj)/qconf - $< $(silent) $(Kconfig) + $(Q)$< $(silent) $(Kconfig) gconfig: $(obj)/gconf - $< $(silent) $(Kconfig) + $(Q)$< $(silent) $(Kconfig) menuconfig: $(obj)/mconf - $< $(silent) $(Kconfig) + $(Q)$< $(silent) $(Kconfig) config: $(obj)/conf - $< $(silent) --oldaskconfig $(Kconfig) + $(Q)$< $(silent) --oldaskconfig $(Kconfig) nconfig: $(obj)/nconf - $< $(silent) $(Kconfig) + $(Q)$< $(silent) $(Kconfig) build_menuconfig: $(obj)/mconf @@ -70,12 +70,12 @@ simple-targets := oldconfig allnoconfig allyesconfig allmodconfig \ PHONY += $(simple-targets) $(simple-targets): $(obj)/conf - $< $(silent) --$@ $(Kconfig) + $(Q)$< $(silent) --$@ $(Kconfig) PHONY += savedefconfig defconfig savedefconfig: $(obj)/conf - $< $(silent) --$@=defconfig $(Kconfig) + $(Q)$< $(silent) --$@=defconfig $(Kconfig) defconfig: $(obj)/conf ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG)),) -- Anthony PERARD
[XEN PATCH 08/15] build: use $(filechk, ) for all compat/.xlat/%.lst
Make use of filechk mean that we don't have to use $(move-if-changed,). It also mean that will have sometime "UPD .." in the build output when the target changed, rather than having "GEN ..." all the time when "xlat.lst" happen to have a more recent modification timestamp. While there, replace `grep -v` by `sed '//d'` to avoid an extra fork and pipe when building. Signed-off-by: Anthony PERARD --- xen/include/Makefile | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/xen/include/Makefile b/xen/include/Makefile index 96d5f6f3c8..2e61b50139 100644 --- a/xen/include/Makefile +++ b/xen/include/Makefile @@ -93,15 +93,13 @@ targets += $(patsubst compat/%, compat/.xlat/%, $(headers-y)) $(obj)/compat/.xlat/%.h: $(obj)/compat/%.h $(obj)/compat/.xlat/%.lst $(srctree)/tools/compat-xlat-header.py FORCE $(call if_changed,xlat_headers) -quiet_cmd_xlat_lst = GEN $@ -cmd_xlat_lst = \ - grep -v '^[[:blank:]]*$(pound)' $< | sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,[[:blank:]]+$*\.h[[:blank:]]*$$,,p' >$@.new; \ - $(call move-if-changed,$@.new,$@) +filechk_xlat_lst = \ + sed -ne '/^[[:blank:]]*$(pound)/d' -e 's,@arch@,$(compat-arch-y),g' -re 's,[[:blank:]]+$*\.h[[:blank:]]*$$,,p' $< .PRECIOUS: $(obj)/compat/.xlat/%.lst targets += $(patsubst compat/%.h, compat/.xlat/%.lst, $(headers-y)) $(obj)/compat/.xlat/%.lst: $(srcdir)/xlat.lst FORCE - $(call if_changed,xlat_lst) + $(call filechk,xlat_lst) xlat-y := $(shell sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,^[?!][[:blank:]]+[^[:blank:]]+[[:blank:]]+,,p' $(srcdir)/xlat.lst | uniq) xlat-y := $(filter $(patsubst compat/%,%,$(headers-y)),$(xlat-y)) -- Anthony PERARD
[XEN PATCH 06/15] build: quiet for .allconfig.tmp target
Signed-off-by: Anthony PERARD --- xen/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/Makefile b/xen/Makefile index e89fc461fc..27f70d2200 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -339,7 +339,7 @@ filechk_kconfig_allconfig = \ : .allconfig.tmp: FORCE - set -e; { $(call filechk_kconfig_allconfig); } > $@ + $(Q)set -e; { $(call filechk_kconfig_allconfig); } > $@ config: tools_fixdep outputmakefile FORCE $(Q)$(MAKE) $(build)=tools/kconfig $@ -- Anthony PERARD
[XEN PATCH 03/15] build, x86: clean build log for boot/ targets
We are adding %.lnk to .PRECIOUS or make treat them as intermediate targets and remove them. Signed-off-by: Anthony PERARD --- xen/arch/x86/boot/Makefile | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile index 03d8ce3a9e..2693b938bd 100644 --- a/xen/arch/x86/boot/Makefile +++ b/xen/arch/x86/boot/Makefile @@ -5,6 +5,8 @@ head-bin-objs := cmdline.o reloc.o nocov-y += $(head-bin-objs) noubsan-y += $(head-bin-objs) targets += $(head-bin-objs) +targets += $(head-bin-objs:.o=.bin) +targets += $(head-bin-objs:.o=.lnk) head-bin-objs := $(addprefix $(obj)/,$(head-bin-objs)) @@ -26,10 +28,16 @@ $(head-bin-objs): XEN_CFLAGS := $(CFLAGS_x86_32) -fpic LDFLAGS_DIRECT-$(call ld-option,--warn-rwx-segments) := --no-warn-rwx-segments LDFLAGS_DIRECT += $(LDFLAGS_DIRECT-y) -%.bin: %.lnk - $(OBJCOPY) -j .text -O binary $< $@ +%.bin: OBJCOPYFLAGS := -j .text -O binary +%.bin: %.lnk FORCE + $(call if_changed,objcopy) -%.lnk: %.o $(src)/build32.lds - $(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT)) -N -T $(filter %.lds,$^) -o $@ $< +quiet_cmd_ld_lnk_o = LD $@ +cmd_ld_lnk_o = $(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT)) -N -T $(filter %.lds,$^) -o $@ $< + +%.lnk: %.o $(src)/build32.lds FORCE + $(call if_changed,ld_lnk_o) clean-files := *.lnk *.bin + +.PRECIOUS: %.lnk -- Anthony PERARD
[XEN PATCH 07/15] build: move XEN_HAS_BUILD_ID out of Config.mk
Whether or not the linker can do build id is only used by the hypervisor build, so move that there. Rename $(build_id_linker) to $(XEN_LDFLAGS_BUILD_ID) as this is a better name to be exported as to use the "XEN_*" namespace. Also update XEN_TREEWIDE_CFLAGS so flags can be used for arch/x86/boot/ CFLAGS_x86_32 Beside a reordering of the command line where CFLAGS is used, there shouldn't be any other changes. Signed-off-by: Anthony PERARD --- Config.mk | 12 xen/Makefile| 12 xen/arch/arm/Makefile | 2 +- xen/arch/riscv/Makefile | 2 +- xen/arch/x86/Makefile | 12 ++-- xen/scripts/Kbuild.include | 3 +++ xen/test/livepatch/Makefile | 4 ++-- 7 files changed, 25 insertions(+), 22 deletions(-) diff --git a/Config.mk b/Config.mk index d12d4c2b8f..27f48f654a 100644 --- a/Config.mk +++ b/Config.mk @@ -125,18 +125,6 @@ endef check-$(gcc) = $(call cc-ver-check,CC,0x040100,"Xen requires at least gcc-4.1") $(eval $(check-y)) -ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \ - grep -q build-id && echo n || echo y) - -export XEN_HAS_BUILD_ID ?= n -ifeq ($(call ld-ver-build-id,$(LD)),n) -build_id_linker := -else -CFLAGS += -DBUILD_ID -export XEN_HAS_BUILD_ID=y -build_id_linker := --build-id=sha1 -endif - define buildmakevars2shellvars export PREFIX="$(prefix)";\ export XEN_SCRIPT_DIR="$(XEN_SCRIPT_DIR)";\ diff --git a/xen/Makefile b/xen/Makefile index 27f70d2200..4dc960df2c 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -286,6 +286,18 @@ CFLAGS += $(CLANG_FLAGS) export CLANG_FLAGS endif +# XEN_HAS_BUILD_ID needed by Kconfig +ifeq ($(call ld-ver-build-id,$(LD)),n) +XEN_LDFLAGS_BUILD_ID := +XEN_HAS_BUILD_ID := n +else +CFLAGS += -DBUILD_ID +XEN_TREEWIDE_CFLAGS += -DBUILD_ID +XEN_HAS_BUILD_ID := y +XEN_LDFLAGS_BUILD_ID := --build-id=sha1 +endif +export XEN_HAS_BUILD_ID XEN_LDFLAGS_BUILD_ID + export XEN_HAS_CHECKPOLICY := $(call success,$(CHECKPOLICY) -h 2>&1 | grep -q xen) # === diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 4d076b278b..1cc57d2cf0 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -102,7 +102,7 @@ $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds $(NM) -pa --format=sysv $(@D)/.$(@F).1 \ | $(objtree)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1.S $(MAKE) $(build)=$(@D) $(@D)/.$(@F).1.o - $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \ + $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(XEN_LDFLAGS_BUILD_ID) \ $(@D)/.$(@F).1.o -o $@ $(NM) -pa --format=sysv $(@D)/$(@F) \ | $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \ diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile index 443f6bf15f..8a0e483c66 100644 --- a/xen/arch/riscv/Makefile +++ b/xen/arch/riscv/Makefile @@ -9,7 +9,7 @@ $(TARGET): $(TARGET)-syms $(OBJCOPY) -O binary -S $< $@ $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds - $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) -o $@ + $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(XEN_LDFLAGS_BUILD_ID) -o $@ $(NM) -pa --format=sysv $(@D)/$(@F) \ | $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \ >$(@D)/$(@F).map diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 2672d7f4ee..c7ec315fe6 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -100,7 +100,7 @@ efi-y := $(shell if [ ! -r $(objtree)/include/xen/compile.h -o \ $(space) efi-$(CONFIG_PV_SHIM_EXCLUSIVE) := -ifneq ($(build_id_linker),) +ifneq ($(XEN_LDFLAGS_BUILD_ID),) notes_phdrs = --notes else ifeq ($(CONFIG_PVH_GUEST),y) @@ -136,19 +136,19 @@ $(TARGET): $(TARGET)-syms $(efi-y) $(obj)/boot/mkelf32 CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds - $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \ + $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(XEN_LDFLAGS_BUILD_ID) \ $(objtree)/common/symbols-dummy.o -o $(@D)/.$(@F).0 $(NM) -pa --format=sysv $(@D)/.$(@F).0 \ | $(objtree)/tools/symbols $(all_symbols) --sysv --sort \ >$(@D)/.$(@F).0.S $(MAKE) $(build)=$(@D) $(@D)/.$(@F).0.o - $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \ + $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(XEN_LDFLAGS_BUILD_ID) \ $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1 $(NM) -pa --format=sysv $(@D)/.$(@F).1 \ | $(objtree)/tools/symbols $(all_symbols) --sysv --sort $(syms-warn-dup-y) \ >$(@D)/.$(@F).1.S $(MAKE) $(build)=$(@D) $(@D)/.$(@F).1.o -
[XEN PATCH 04/15] build: hide policy.bin commands
Instead, show only when "policy.bin" is been updated. We still have the full command from the flask/policy Makefile, but we can't change that Makefile. Signed-off-by: Anthony PERARD --- xen/xsm/flask/Makefile | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile index 3fdcf7727e..fc44ad684f 100644 --- a/xen/xsm/flask/Makefile +++ b/xen/xsm/flask/Makefile @@ -48,10 +48,15 @@ targets += flask-policy.S FLASK_BUILD_DIR := $(abs_objtree)/$(obj) POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION) +policy_chk = \ +$(Q)if ! cmp -s $(POLICY_SRC) $@; then \ +$(kecho) ' UPD $@'; \ +cp $(POLICY_SRC) $@; \ +fi $(obj)/policy.bin: FORCE - $(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common \ + $(Q)$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common \ -C $(XEN_ROOT)/tools/flask/policy \ FLASK_BUILD_DIR=$(FLASK_BUILD_DIR) POLICY_FILENAME=$(POLICY_SRC) - cmp -s $(POLICY_SRC) $@ || cp $(POLICY_SRC) $@ + $(call policy_chk) clean-files := policy.* $(POLICY_SRC) -- Anthony PERARD
[XEN PATCH 01/15] build: hide that we are updating xen/lib/x86
Change of directory to xen/lib/x86 isn't needed to be shown. If something gets updated, make will print the command line. Signed-off-by: Anthony PERARD --- xen/include/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/include/Makefile b/xen/include/Makefile index edd5322e88..96d5f6f3c8 100644 --- a/xen/include/Makefile +++ b/xen/include/Makefile @@ -229,7 +229,7 @@ ifeq ($(XEN_TARGET_ARCH),x86_64) .PHONY: lib-x86-all lib-x86-all: @mkdir -p $(obj)/xen/lib/x86 - $(MAKE) -C $(obj)/xen/lib/x86 -f $(abs_srctree)/$(src)/xen/lib/x86/Makefile all + $(Q)$(MAKE) -C $(obj)/xen/lib/x86 -f $(abs_srctree)/$(src)/xen/lib/x86/Makefile all all: lib-x86-all endif -- Anthony PERARD
[XEN PATCH 00/15] build: cleanup build log, avoid user's CFLAGS, avoid too many include of Config.mk
Patch series available in this git branch: https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.build-system-xen-removing-config.mk-v1 Hi, This series of patch cleanup the remaining rules still displaying their command line. Then, some change are made in Config.mk to remove build-id stuff only used by Xen build. Then, the variable AFLAGS and CFLAGS are been renamed XEN_AFLAGS and XEN_CFLAGS from the beginning to about inclusion of users CFLAGS as those are usually made for user space program, not kernel, especially in build environment for distro packages. Last patch removes the inclusion of Config.mk from xen/Rules.mk, as this slow down the build unnecessarily. xen/Makefile should do everything necessary to setup the build of the hypervisor, and is its only entry point. Thanks, Anthony PERARD (15): build: hide that we are updating xen/lib/x86 build: rework asm-offsets.* build step to use kbuild build, x86: clean build log for boot/ targets build: hide policy.bin commands build: introduce a generic command for gzip build: quiet for .allconfig.tmp target build: move XEN_HAS_BUILD_ID out of Config.mk build: use $(filechk, ) for all compat/.xlat/%.lst build: hide commands run for kconfig build: rename $(AFLAGS) to $(XEN_AFLAGS) build: rename CFLAGS to XEN_CFLAGS in xen/Makefile build: avoid Config.mk's CFLAGS build: fix compile.h compiler version command line Config.mk: move $(cc-option, ) to config/compiler-testing.mk build: remove Config.mk include from Rules.mk Config.mk | 39 +-- config/compiler-testing.mk | 25 + xen/Makefile| 75 + xen/Rules.mk| 7 +++- xen/arch/arm/Makefile | 2 +- xen/arch/arm/arch.mk| 8 +++- xen/arch/riscv/Makefile | 2 +- xen/arch/riscv/arch.mk | 4 +- xen/arch/x86/Makefile | 12 +++--- xen/arch/x86/arch.mk| 62 +++--- xen/arch/x86/boot/Makefile | 16 ++-- xen/build.mk| 24 +++- xen/common/Makefile | 8 ++-- xen/include/Makefile| 10 ++--- xen/scripts/Kbuild.include | 10 + xen/test/livepatch/Makefile | 4 +- xen/tools/kconfig/Makefile | 14 +++ xen/xsm/flask/Makefile | 9 - 18 files changed, 193 insertions(+), 138 deletions(-) create mode 100644 config/compiler-testing.mk -- Anthony PERARD
[XEN PATCH 02/15] build: rework asm-offsets.* build step to use kbuild
Use $(if_changed_dep, ) macro to generate "asm-offsets.s" and remove the use of $(move-if-changes,). That mean that "asm-offset.s" will be changed even when the output doesn't change. But "asm-offsets.s" is only used to generated "asm-offsets.h". So instead of regenerating "asm-offsets.h" every time "asm-offsets.s" change, we will use "$(filechk, )" to only update the ".h" when the output change. Also, with "$(filechk, )", the file does get regenerated when the rule change in the makefile. This changes also result in a cleaner build log. Signed-off-by: Anthony PERARD --- Instead of having a special $(cmd_asm-offsets.s) command, we could probably reuse $(cmd_cc_s_c) from Rules.mk, but that would mean that an hypothetical additional flags "-flto" in CFLAGS would not be removed anymore, not sure if that matter here. But then we could write this: targets += arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s: CFLAGS-y += -g0 arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s FORCE instead of having to write a rule for asm-offsets.s --- xen/build.mk | 22 ++ 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/xen/build.mk b/xen/build.mk index 758590c68e..e2a78aa806 100644 --- a/xen/build.mk +++ b/xen/build.mk @@ -40,13 +40,15 @@ include/xen/compile.h: include/xen/compile.h.in .banner FORCE targets += include/xen/compile.h --include $(wildcard .asm-offsets.s.d) -asm-offsets.s: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.c - $(CC) $(call cpp_flags,$(c_flags)) -S -g0 -o $@.new -MQ $@ $< - $(call move-if-changed,$@.new,$@) +quiet_cmd_asm-offsets.s = CC $@ +cmd_asm-offsets.s = $(CC) $(call cpp_flags,$(c_flags)) -S -g0 $< -o $@ -arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: asm-offsets.s - @(set -e; \ +asm-offsets.s: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.c FORCE + $(call if_changed_dep,asm-offsets.s) + +targets += asm-offsets.s + +define filechk_asm-offsets.h echo "/*"; \ echo " * DO NOT MODIFY."; \ echo " *"; \ @@ -57,9 +59,13 @@ arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: asm-offsets.s echo "#ifndef __ASM_OFFSETS_H__"; \ echo "#define __ASM_OFFSETS_H__"; \ echo ""; \ - sed -rne "/^[^#].*==>/{s:.*==>(.*)<==.*:\1:; s: [\$$#]: :; p;}"; \ + sed -rne "/^[^#].*==>/{s:.*==>(.*)<==.*:\1:; s: [\$$#]: :; p;}" $<; \ echo ""; \ - echo "#endif") <$< >$@ + echo "#endif" +endef + +arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: asm-offsets.s FORCE + $(call filechk,asm-offsets.h) build-dirs := $(patsubst %/built_in.o,%,$(filter %/built_in.o,$(ALL_OBJS) $(ALL_LIBS))) -- Anthony PERARD
[libvirt test] 180910: tolerable all pass - PUSHED
flight 180910 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/180910/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 180714 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 180714 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 180714 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-libvirt-qcow2 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 15 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass version targeted for testing: libvirt 3b6d69237f0a07bf8d9807cd68a387f8d42b076f baseline version: libvirt 90404c53682f464b4a26efd618887dc336d9da80 Last test of basis 180714 2023-05-19 04:18:50 Z4 days Testing same since 180910 2023-05-23 04:18:51 Z0 days1 attempts People who touched revisions under test: Andrea Bolognani Michal Privoznik jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-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-arm64-arm64-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt pass test-armhf-armhf-libvirt pass test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair pass test-arm64-arm64-libvirt-qcow2 pass test-armhf-armhf-libvirt-qcow2 pass test-arm64-arm64-libvirt-raw pass test-armhf-armhf-libvirt-raw pass test-amd64-i386-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
Re: [PATCH 3/6] block/blkio: convert to blk_io_plug_call() API
On Fri, May 19, 2023 at 10:47:00AM +0200, Stefano Garzarella wrote: > On Wed, May 17, 2023 at 06:10:19PM -0400, Stefan Hajnoczi wrote: > > Stop using the .bdrv_co_io_plug() API because it is not multi-queue > > block layer friendly. Use the new blk_io_plug_call() API to batch I/O > > submission instead. > > > > Signed-off-by: Stefan Hajnoczi > > --- > > block/blkio.c | 40 +--- > > 1 file changed, 21 insertions(+), 19 deletions(-) > > With this patch, the build fails in several places, maybe it's an old > version: > > ../block/blkio.c:347:5: error: implicit declaration of function > ‘blk_io_plug_call’ [-Werror=implicit-function-declaration] > 347 | blk_io_plug_call(blkio_unplug_fn, bs); Will fix, thanks! Stefan signature.asc Description: PGP signature
Re: [PATCH 4/6] block/io_uring: convert to blk_io_plug_call() API
On Thu, May 18, 2023 at 07:18:42PM -0500, Eric Blake wrote: > On Wed, May 17, 2023 at 06:10:20PM -0400, Stefan Hajnoczi wrote: > > Stop using the .bdrv_co_io_plug() API because it is not multi-queue > > block layer friendly. Use the new blk_io_plug_call() API to batch I/O > > submission instead. > > > > Signed-off-by: Stefan Hajnoczi > > --- > > include/block/raw-aio.h | 7 --- > > block/file-posix.c | 10 - > > block/io_uring.c| 45 - > > block/trace-events | 5 ++--- > > 4 files changed, 19 insertions(+), 48 deletions(-) > > > > > @@ -337,7 +325,6 @@ void luring_io_unplug(void) > > * @type: type of request > > * > > * Fetches sqes from ring, adds to pending queue and preps them > > - * > > */ > > static int luring_do_submit(int fd, LuringAIOCB *luringcb, LuringState *s, > > uint64_t offset, int type) > > @@ -370,14 +357,16 @@ static int luring_do_submit(int fd, LuringAIOCB > > *luringcb, LuringState *s, > > Looks a bit like a stray hunk, but you are touching the function, so > it's okay. I'm respinning, so I'll drop this. Stefan signature.asc Description: PGP signature
Re: [PATCH 1/6] block: add blk_io_plug_call() API
On Thu, May 18, 2023 at 07:04:52PM -0500, Eric Blake wrote: > On Wed, May 17, 2023 at 06:10:17PM -0400, Stefan Hajnoczi wrote: > > Introduce a new API for thread-local blk_io_plug() that does not > > traverse the block graph. The goal is to make blk_io_plug() multi-queue > > friendly. > > > > Instead of having block drivers track whether or not we're in a plugged > > section, provide an API that allows them to defer a function call until > > we're unplugged: blk_io_plug_call(fn, opaque). If blk_io_plug_call() is > > called multiple times with the same fn/opaque pair, then fn() is only > > called once at the end of the function - resulting in batching. > > > > This patch introduces the API and changes blk_io_plug()/blk_io_unplug(). > > blk_io_plug()/blk_io_unplug() no longer require a BlockBackend argument > > because the plug state is now thread-local. > > > > Later patches convert block drivers to blk_io_plug_call() and then we > > can finally remove .bdrv_co_io_plug() once all block drivers have been > > converted. > > > > Signed-off-by: Stefan Hajnoczi > > --- > > > +++ b/block/plug.c > > + > > +/** > > + * blk_io_plug_call: > > + * @fn: a function pointer to be invoked > > + * @opaque: a user-defined argument to @fn() > > + * > > + * Call @fn(@opaque) immediately if not within a > > blk_io_plug()/blk_io_unplug() > > + * section. > > + * > > + * Otherwise defer the call until the end of the outermost > > + * blk_io_plug()/blk_io_unplug() section in this thread. If the same > > + * @fn/@opaque pair has already been deferred, it will only be called once > > upon > > + * blk_io_unplug() so that accumulated calls are batched into a single > > call. > > + * > > + * The caller must ensure that @opaque is not be freed before @fn() is > > invoked. > > s/be // Will fix, thanks! Stefan signature.asc Description: PGP signature
Re: [PATCH 2/6] block/nvme: convert to blk_io_plug_call() API
On Fri, May 19, 2023 at 10:46:25AM +0200, Stefano Garzarella wrote: > On Wed, May 17, 2023 at 06:10:18PM -0400, Stefan Hajnoczi wrote: > > Stop using the .bdrv_co_io_plug() API because it is not multi-queue > > block layer friendly. Use the new blk_io_plug_call() API to batch I/O > > submission instead. > > > > Signed-off-by: Stefan Hajnoczi > > --- > > block/nvme.c | 44 > > 1 file changed, 12 insertions(+), 32 deletions(-) > > > > diff --git a/block/nvme.c b/block/nvme.c > > index 5b744c2bda..100b38b592 100644 > > --- a/block/nvme.c > > +++ b/block/nvme.c > > @@ -25,6 +25,7 @@ > > #include "qemu/vfio-helpers.h" > > #include "block/block-io.h" > > #include "block/block_int.h" > > +#include "sysemu/block-backend.h" > > #include "sysemu/replay.h" > > #include "trace.h" > > > > @@ -119,7 +120,6 @@ struct BDRVNVMeState { > > int blkshift; > > > > uint64_t max_transfer; > > -bool plugged; > > > > bool supports_write_zeroes; > > bool supports_discard; > > @@ -282,7 +282,7 @@ static void nvme_kick(NVMeQueuePair *q) > > { > > BDRVNVMeState *s = q->s; > > > > -if (s->plugged || !q->need_kick) { > > +if (!q->need_kick) { > > return; > > } > > trace_nvme_kick(s, q->index); > > @@ -387,10 +387,6 @@ static bool nvme_process_completion(NVMeQueuePair *q) > > NvmeCqe *c; > > > > trace_nvme_process_completion(s, q->index, q->inflight); > > -if (s->plugged) { > > -trace_nvme_process_completion_queue_plugged(s, q->index); > > Should we remove "nvme_process_completion_queue_plugged(void *s, > unsigned q_index) "s %p q #%u" from block/trace-events? Will fix, thanks! Stefan signature.asc Description: PGP signature
Re: [PATCH 1/6] block: add blk_io_plug_call() API
On Fri, May 19, 2023 at 10:45:57AM +0200, Stefano Garzarella wrote: > On Wed, May 17, 2023 at 06:10:17PM -0400, Stefan Hajnoczi wrote: > > Introduce a new API for thread-local blk_io_plug() that does not > > traverse the block graph. The goal is to make blk_io_plug() multi-queue > > friendly. > > > > Instead of having block drivers track whether or not we're in a plugged > > section, provide an API that allows them to defer a function call until > > we're unplugged: blk_io_plug_call(fn, opaque). If blk_io_plug_call() is > > called multiple times with the same fn/opaque pair, then fn() is only > > called once at the end of the function - resulting in batching. > > > > This patch introduces the API and changes blk_io_plug()/blk_io_unplug(). > > blk_io_plug()/blk_io_unplug() no longer require a BlockBackend argument > > because the plug state is now thread-local. > > > > Later patches convert block drivers to blk_io_plug_call() and then we > > can finally remove .bdrv_co_io_plug() once all block drivers have been > > converted. > > > > Signed-off-by: Stefan Hajnoczi > > --- > > MAINTAINERS | 1 + > > include/sysemu/block-backend-io.h | 13 +-- > > block/block-backend.c | 22 - > > block/plug.c | 159 ++ > > hw/block/dataplane/xen-block.c| 8 +- > > hw/block/virtio-blk.c | 4 +- > > hw/scsi/virtio-scsi.c | 6 +- > > block/meson.build | 1 + > > 8 files changed, 173 insertions(+), 41 deletions(-) > > create mode 100644 block/plug.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 50585117a0..574202295c 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -2644,6 +2644,7 @@ F: util/aio-*.c > > F: util/aio-*.h > > F: util/fdmon-*.c > > F: block/io.c > > +F: block/plug.c > > F: migration/block* > > F: include/block/aio.h > > F: include/block/aio-wait.h > > diff --git a/include/sysemu/block-backend-io.h > > b/include/sysemu/block-backend-io.h > > index d62a7ee773..be4dcef59d 100644 > > --- a/include/sysemu/block-backend-io.h > > +++ b/include/sysemu/block-backend-io.h > > @@ -100,16 +100,9 @@ void blk_iostatus_set_err(BlockBackend *blk, int > > error); > > int blk_get_max_iov(BlockBackend *blk); > > int blk_get_max_hw_iov(BlockBackend *blk); > > > > -/* > > - * blk_io_plug/unplug are thread-local operations. This means that multiple > > - * IOThreads can simultaneously call plug/unplug, but the caller must > > ensure > > - * that each unplug() is called in the same IOThread of the matching > > plug(). > > - */ > > -void coroutine_fn blk_co_io_plug(BlockBackend *blk); > > -void co_wrapper blk_io_plug(BlockBackend *blk); > > - > > -void coroutine_fn blk_co_io_unplug(BlockBackend *blk); > > -void co_wrapper blk_io_unplug(BlockBackend *blk); > > +void blk_io_plug(void); > > +void blk_io_unplug(void); > > +void blk_io_plug_call(void (*fn)(void *), void *opaque); > > > > AioContext *blk_get_aio_context(BlockBackend *blk); > > BlockAcctStats *blk_get_stats(BlockBackend *blk); > > diff --git a/block/block-backend.c b/block/block-backend.c > > index ca537cd0ad..1f1d226ba6 100644 > > --- a/block/block-backend.c > > +++ b/block/block-backend.c > > @@ -2568,28 +2568,6 @@ void blk_add_insert_bs_notifier(BlockBackend *blk, > > Notifier *notify) > > notifier_list_add(>insert_bs_notifiers, notify); > > } > > > > -void coroutine_fn blk_co_io_plug(BlockBackend *blk) > > -{ > > -BlockDriverState *bs = blk_bs(blk); > > -IO_CODE(); > > -GRAPH_RDLOCK_GUARD(); > > - > > -if (bs) { > > -bdrv_co_io_plug(bs); > > -} > > -} > > - > > -void coroutine_fn blk_co_io_unplug(BlockBackend *blk) > > -{ > > -BlockDriverState *bs = blk_bs(blk); > > -IO_CODE(); > > -GRAPH_RDLOCK_GUARD(); > > - > > -if (bs) { > > -bdrv_co_io_unplug(bs); > > -} > > -} > > - > > BlockAcctStats *blk_get_stats(BlockBackend *blk) > > { > > IO_CODE(); > > diff --git a/block/plug.c b/block/plug.c > > new file mode 100644 > > index 00..6738a568ba > > --- /dev/null > > +++ b/block/plug.c > > @@ -0,0 +1,159 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > +/* > > + * Block I/O plugging > > + * > > + * Copyright Red Hat. > > + * > > + * This API defers a function call within a blk_io_plug()/blk_io_unplug() > > + * section, allowing multiple calls to batch up. This is a performance > > + * optimization that is used in the block layer to submit several I/O > > requests > > + * at once instead of individually: > > + * > > + * blk_io_plug(); <-- start of plugged region > > + * ... > > + * blk_io_plug_call(my_func, my_obj); <-- deferred my_func(my_obj) call > > + * blk_io_plug_call(my_func, my_obj); <-- another > > + * blk_io_plug_call(my_func, my_obj); <-- another > > + * ... > > + * blk_io_unplug(); <-- end of plugged region, my_func(my_obj) is called > > once > > + * > > + * This code is actually generic and not tied to the
[OSSTEST PATCH] ts-xen-build-prep: Install python3-venv for QEMU
Since QEMU commit 81e2b198a8cb ("configure: create a python venv unconditionally"), a python venv is always created and this new package is needed on Debian. Signed-off-by: Anthony PERARD --- ts-xen-build-prep | 1 + 1 file changed, 1 insertion(+) diff --git a/ts-xen-build-prep b/ts-xen-build-prep index 3ae8f215..547bbc16 100755 --- a/ts-xen-build-prep +++ b/ts-xen-build-prep @@ -209,6 +209,7 @@ sub prep () { libdevmapper-dev libxml-xpath-perl libelf-dev ccache nasm checkpolicy ebtables python3-docutils python3-dev + python3-venv libtirpc-dev libgnutls28-dev); -- Anthony PERARD
Re: [PATCH] xen/netback: Pass (void *) to virt_to_page()
On Tue, May 23, 2023 at 04:03:42PM +0200, Linus Walleij wrote: > virt_to_page() takes a virtual address as argument but > the driver passes an unsigned long, which works because > the target platform(s) uses polymorphic macros to calculate > the page. > > Since many architectures implement virt_to_pfn() as > a macro, this function becomes polymorphic and accepts both a > (unsigned long) and a (void *). > > Fix this up by an explicit (void *) cast. > > Cc: Wei Liu > Cc: Paul Durrant > Cc: xen-devel@lists.xenproject.org > Cc: net...@vger.kernel.org > Signed-off-by: Linus Walleij Acked-by: Wei Liu
Re: [RFC] Xen crashes on ASSERT on suspend/resume, suggested fix
On Tue, May 23, 2023 at 03:54:36PM +0200, Roger Pau Monné wrote: > On Thu, May 18, 2023 at 04:44:53PM -0700, Stefano Stabellini wrote: > > Hi all, > > > > After many PVH Dom0 suspend/resume cycles we are seeing the following > > Xen crash (it is random and doesn't reproduce reliably): > > > > (XEN) [555.042981][] R > > arch/x86/irq.c#_clear_irq_vector+0x214/0x2bd > > (XEN) [555.042986][] F destroy_irq+0xe2/0x1b8 > > (XEN) [555.042991][] F msi_free_irq+0x5e/0x1a7 > > (XEN) [555.042995][] F unmap_domain_pirq+0x441/0x4b4 > > (XEN) [555.043001][] F > > arch/x86/hvm/vmsi.c#vpci_msi_disable+0xc0/0x155 > > (XEN) [555.043006][] F vpci_msi_arch_disable+0x1e/0x2b > > (XEN) [555.043013][] F > > drivers/vpci/msi.c#control_write+0x109/0x10e > > (XEN) [555.043018][] F vpci_write+0x11f/0x268 > > (XEN) [555.043024][] F > > arch/x86/hvm/io.c#vpci_portio_write+0xa0/0xa7 > > (XEN) [555.043029][] F > > hvm_process_io_intercept+0x203/0x26f > > (XEN) [555.043034][] F hvm_io_intercept+0x2a/0x4c > > (XEN) [555.043039][] F > > arch/x86/hvm/emulate.c#hvmemul_do_io+0x29b/0x5f6 > > (XEN) [555.043043][] F > > arch/x86/hvm/emulate.c#hvmemul_do_io_buffer+0x2f/0x6a > > (XEN) [555.043048][] F hvmemul_do_pio_buffer+0x33/0x35 > > (XEN) [555.043053][] F handle_pio+0x6d/0x1b4 > > (XEN) [555.043059][] F svm_vmexit_handler+0x10bf/0x18b0 > > (XEN) [555.043064][] F svm_stgi_label+0x8/0x18 > > (XEN) [555.043067] > > (XEN) [555.469861] > > (XEN) [555.471855] > > (XEN) [555.477315] Panic on CPU 9: > > (XEN) [555.480608] Assertion 'per_cpu(vector_irq, cpu)[old_vector] == irq' > > failed at arch/x86/irq.c:233 > > (XEN) [555.489882] > > > > Looking at the code in question, the ASSERT looks wrong to me. > > > > Specifically, if you see send_cleanup_vector and > > irq_move_cleanup_interrupt, it is entirely possible to have old_vector > > still valid and also move_in_progress still set, but only some of the > > per_cpu(vector_irq, me)[vector] cleared. It seems to me that this could > > happen especially when an MSI has a large old_cpu_mask. > > i guess the only way to get into such situation would be if you happen > to execute _clear_irq_vector() with a cpu_online_map smaller than the > one in old_cpu_mask, at which point you will leave old_vector fields > not updated. > > Maybe somehow you get into this situation when doing suspend/resume? > > Could you try to add a: > > ASSERT(cpumask_equal(tmp_mask, desc->arch.old_cpu_mask)); > > Before the `for_each_cpu(cpu, tmp_mask)` loop? I see that the old_cpu_mask is cleared in release_old_vec(), so that suggestion is not very useful. Does the crash happen at specific points, for example just after resume or before suspend? Roger.
[PATCH] xen/netback: Pass (void *) to virt_to_page()
virt_to_page() takes a virtual address as argument but the driver passes an unsigned long, which works because the target platform(s) uses polymorphic macros to calculate the page. Since many architectures implement virt_to_pfn() as a macro, this function becomes polymorphic and accepts both a (unsigned long) and a (void *). Fix this up by an explicit (void *) cast. Cc: Wei Liu Cc: Paul Durrant Cc: xen-devel@lists.xenproject.org Cc: net...@vger.kernel.org Signed-off-by: Linus Walleij --- drivers/net/xen-netback/netback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c index c1501f41e2d8..caf0c815436c 100644 --- a/drivers/net/xen-netback/netback.c +++ b/drivers/net/xen-netback/netback.c @@ -689,7 +689,7 @@ static void xenvif_fill_frags(struct xenvif_queue *queue, struct sk_buff *skb) prev_pending_idx = pending_idx; txp = >pending_tx_info[pending_idx].req; - page = virt_to_page(idx_to_kaddr(queue, pending_idx)); + page = virt_to_page((void *)idx_to_kaddr(queue, pending_idx)); __skb_fill_page_desc(skb, i, page, txp->offset, txp->size); skb->len += txp->size; skb->data_len += txp->size; -- 2.34.1
Re: [RFC] Xen crashes on ASSERT on suspend/resume, suggested fix
On Thu, May 18, 2023 at 04:44:53PM -0700, Stefano Stabellini wrote: > Hi all, > > After many PVH Dom0 suspend/resume cycles we are seeing the following > Xen crash (it is random and doesn't reproduce reliably): > > (XEN) [555.042981][] R > arch/x86/irq.c#_clear_irq_vector+0x214/0x2bd > (XEN) [555.042986][] F destroy_irq+0xe2/0x1b8 > (XEN) [555.042991][] F msi_free_irq+0x5e/0x1a7 > (XEN) [555.042995][] F unmap_domain_pirq+0x441/0x4b4 > (XEN) [555.043001][] F > arch/x86/hvm/vmsi.c#vpci_msi_disable+0xc0/0x155 > (XEN) [555.043006][] F vpci_msi_arch_disable+0x1e/0x2b > (XEN) [555.043013][] F > drivers/vpci/msi.c#control_write+0x109/0x10e > (XEN) [555.043018][] F vpci_write+0x11f/0x268 > (XEN) [555.043024][] F > arch/x86/hvm/io.c#vpci_portio_write+0xa0/0xa7 > (XEN) [555.043029][] F hvm_process_io_intercept+0x203/0x26f > (XEN) [555.043034][] F hvm_io_intercept+0x2a/0x4c > (XEN) [555.043039][] F > arch/x86/hvm/emulate.c#hvmemul_do_io+0x29b/0x5f6 > (XEN) [555.043043][] F > arch/x86/hvm/emulate.c#hvmemul_do_io_buffer+0x2f/0x6a > (XEN) [555.043048][] F hvmemul_do_pio_buffer+0x33/0x35 > (XEN) [555.043053][] F handle_pio+0x6d/0x1b4 > (XEN) [555.043059][] F svm_vmexit_handler+0x10bf/0x18b0 > (XEN) [555.043064][] F svm_stgi_label+0x8/0x18 > (XEN) [555.043067] > (XEN) [555.469861] > (XEN) [555.471855] > (XEN) [555.477315] Panic on CPU 9: > (XEN) [555.480608] Assertion 'per_cpu(vector_irq, cpu)[old_vector] == irq' > failed at arch/x86/irq.c:233 > (XEN) [555.489882] > > Looking at the code in question, the ASSERT looks wrong to me. > > Specifically, if you see send_cleanup_vector and > irq_move_cleanup_interrupt, it is entirely possible to have old_vector > still valid and also move_in_progress still set, but only some of the > per_cpu(vector_irq, me)[vector] cleared. It seems to me that this could > happen especially when an MSI has a large old_cpu_mask. i guess the only way to get into such situation would be if you happen to execute _clear_irq_vector() with a cpu_online_map smaller than the one in old_cpu_mask, at which point you will leave old_vector fields not updated. Maybe somehow you get into this situation when doing suspend/resume? Could you try to add a: ASSERT(cpumask_equal(tmp_mask, desc->arch.old_cpu_mask)); Before the `for_each_cpu(cpu, tmp_mask)` loop? Thanks, Roger.
Re: [PATCH v2] maintainers: add regex matching for xsm
Hi Daniel, On 22/05/2023 20:14, Daniel P. Smith wrote: XSM is a subsystem where it is equally important of how and where its hooks are called as is the implementation of the hooks. The people best suited for evaluating the how and where are the XSM maintainers and reviewers. This creates a challenge as the hooks are used throughout the hypervisor for which the XSM maintainers and reviewers are not, and should not be, a reviewer for each of these subsystems in the MAINTAINERS file. Though the MAINTAINERS file does support the use of regex matches, 'K' identifier, that are applied to both the commit message and the commit delta. Adding the 'K' identifier will declare that any patch relating to XSM require the input from the XSM maintainers and reviewers. For those that use the get_maintianers script, the 'K' identifier will automatically add the XSM maintainers and reviewers. Any one not using get_maintainers, it will be their responsibility to ensure that if their work touches and XSM hook, to ensure the XSM maintainers and reviewers are copied. This patch adds a pair of regex expressions to the XSM section. The first is `xsm_.*` which seeks to match XSM hooks in the commit's delta. The second is `\b(xsm|XSM)\b` which seeks to match strictly the words xsm or XSM and should not capture words with a substring of "xsm". Signed-off-by: Daniel P. Smith Acked-by: Julien Grall Cheers, --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index f2f1881b32..b0f0823d21 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -674,6 +674,8 @@ F: tools/flask/ F:xen/include/xsm/ F:xen/xsm/ F:docs/misc/xsm-flask.txt +K: xsm_.* +K: \b(xsm|XSM)\b THE REST M:Andrew Cooper -- Julien Grall
Re: [PATCH v2] x86: do away with HAVE_AS_NEGATIVE_TRUE
On 23/05/2023 1:37 pm, Jan Beulich wrote: > There's no real need for the associated probing - we can easily convert > to a uniform value without knowing the specific behavior (note also that > the respective comments weren't fully correct and have gone stale). > > No difference in generated code. > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper Thanks. I do think this form is easier to follow. ~Andrew
Re: PVH Dom0 related UART failure
On 23.05.2023 12:59, Roger Pau Monné wrote: > On Tue, May 23, 2023 at 08:44:48AM +0200, Jan Beulich wrote: >> On 23.05.2023 00:20, Stefano Stabellini wrote: >>> On Sat, 20 May 2023, Roger Pau Monné wrote: diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c index ec2e978a4e6b..0ff8e940fa8d 100644 --- a/xen/drivers/vpci/header.c +++ b/xen/drivers/vpci/header.c @@ -289,6 +289,13 @@ static int modify_bars(const struct pci_dev *pdev, uint16_t cmd, bool rom_only) */ for_each_pdev ( pdev->domain, tmp ) { +if ( !tmp->vpci ) +{ +printk(XENLOG_G_WARNING "%pp: not handled by vPCI for %pd\n", + >sbdf, pdev->domain); +continue; +} + if ( tmp == pdev ) { /* diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c index 652807a4a454..0baef3a8d3a1 100644 --- a/xen/drivers/vpci/vpci.c +++ b/xen/drivers/vpci/vpci.c @@ -72,7 +72,12 @@ int vpci_add_handlers(struct pci_dev *pdev) unsigned int i; int rc = 0; -if ( !has_vpci(pdev->domain) ) +if ( !has_vpci(pdev->domain) || + /* + * Ignore RO and hidden devices, those are in use by Xen and vPCI + * won't work on them. + */ + pci_get_pdev(dom_xen, pdev->sbdf) ) return 0; /* We should not get here twice for the same device. */ >>> >>> >>> Now this patch works! Thank you!! :-) >>> >>> You can check the full logs here >>> https://gitlab.com/xen-project/people/sstabellini/xen/-/jobs/4329259080 >>> >>> Is the patch ready to be upstreamed aside from the commit message? >> >> I don't think so. vPCI ought to work on "r/o" devices. Out of curiosity, >> have you also tried my (hackish and hence RFC) patch [1]? > > For r/o devices there should be no need of vPCI handlers, reading the > config space of such devices can be done directly. > > There's some work to be done for hidden devices, as for those dom0 has > write access to the config space and thus needs vPCI to be setup > properly. But then isn't it going to complicate things when dealing with r/o and hidden devices differently? > The change to modify_bars() in order to handle devices without vpci > populated is a bugfix, as it's already possible to have devices > assigned to a domain that don't have vpci setup, if the call to > vpci_add_handlers() in setup_one_hwdom_device() fails. That one could > go in separately of the rest of the work in order to enable support > for hidden devices. You saying "assigned to a domain" makes this sound more problematic than it probably is: If it really was any domain other than Dom0, I think there would be a security concern. Yet even for Dom0 I wonder what good can come out of there not being proper vPCI setup for a device. Jan
Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature
On 23.05.2023 13:57, Luca Fancellu wrote: >> On 23 May 2023, at 12:53, Jan Beulich wrote: >> On 23.05.2023 13:50, Luca Fancellu wrote: On 23 May 2023, at 11:31, Jan Beulich wrote: On 23.05.2023 12:21, Luca Fancellu wrote: >> On 23 May 2023, at 11:02, Jan Beulich wrote: >> On 23.05.2023 09:43, Luca Fancellu wrote: >>> @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 >>> systems. >>> >>> If using this option is necessary to fix an issue, please report a >>> bug. >>> >>> +Enables features on dom0 on Arm systems. >>> + >>> +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain >>> and sets >>> +the maximum SVE vector length, the option is applicable only to >>> AArch64 >>> +guests. >> >> Why "guests"? Does the option affect more than Dom0? > > I used “guests” because in my mind I was referring to all the aarch64 OS > that can be used > as control domain, I can change it if it sounds bad. If you means OSes then better also say OSes. But maybe this doesn't need specifically expressing, by saying e.g. "..., the option is applicable only on AArch64"? Or can a Dom0 be 32-bit on Arm64 Xen? >>> >>> I think there is no limitation so Dom0 can be 32 bit or 64. Maybe I can say >>> “... AArch64 kernel guests.”? >> >> I'd recommend to avoid the term "guest" when you talk about Dom0 alone. >> Commonly "guest" means ordinary domains only, i.e. in particular excluding >> Dom0. What's wrong with "AArch64 Dom0 kernels"? > > Ok works for me, I will use “AArch64 Dom0 kernels", I thought “guests” were a > generic category > and then we have “privileged guests”, for example Dom0 or driver domain, and > “unprivileged guests” > like DomUs. Well, yes - "commonly" doesn't mean "always". >>> +A value equal to 0 disables the feature, this is the default value. >>> +Values below 0 means the feature uses the maximum SVE vector length >>> +supported by hardware, if SVE is supported. >>> +Values above 0 explicitly set the maximum SVE vector length for >>> Dom0, >>> +allowed values are from 128 to maximum 2048, being multiple of 128. >>> +Please note that when the user explicitly specifies the value, if >>> that value >>> +is above the hardware supported maximum SVE vector length, the >>> domain >>> +creation will fail and the system will stop, the same will occur >>> if the >>> +option is provided with a non zero value, but the platform doesn't >>> support >>> +SVE. >> >> Assuming this also covers the -1 case, I wonder if that isn't a little >> too >> strict. "Maximum supported" imo can very well be 0. > > Maximum supported, when platforms uses SVE, can be at minimum 128 by arm > specs. When there is SVE - sure. But when there's no SVE, 0 is kind of the implied length. And I'd view a command line option value of -1 quite okay in that case: They've asked for the maximum supported, so they'll get 0. No reason to crash the system during boot. >>> >>> Ok I see what you mean, for example when Kconfig SVE is enabled, but the >>> platform doesn’t >>> have SVE feature, requesting sve=-1 will keep the value to 0, and no system >>> will be stopped. >>> >>> Maybe I can say: >>> >>> “... the same will occur if the option is provided with a positive non zero >>> value, >>> but the platform doesn't support SVE." >> >> Right, provided that matches the implementation. > > Ok I will do the changes, can I retain your R-by? I suppose it covers also > documentation right? I guess whether doc is covered is fuzzy. Since the doc part is Arm- specific, I'd probably consider it not covered with the "!arm" that I appended. But whichever way you look at it, you can keep the tag in place. Jan
[PATCH v2] x86: do away with HAVE_AS_NEGATIVE_TRUE
There's no real need for the associated probing - we can easily convert to a uniform value without knowing the specific behavior (note also that the respective comments weren't fully correct and have gone stale). No difference in generated code. Signed-off-by: Jan Beulich --- v2: Use "& 1". --- a/xen/arch/x86/arch.mk +++ b/xen/arch/x86/arch.mk @@ -26,10 +26,6 @@ $(call as-option-add,CFLAGS,CC,"invpcid $(call as-option-add,CFLAGS,CC,"movdiri %rax$(comma)(%rax)",-DHAVE_AS_MOVDIR) $(call as-option-add,CFLAGS,CC,"enqcmd (%rax)$(comma)%rax",-DHAVE_AS_ENQCMD) -# GAS's idea of true is -1. Clang's idea is 1 -$(call as-option-add,CFLAGS,CC,\ -".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE) - # Check to see whether the assmbler supports the .nop directive. $(call as-option-add,CFLAGS,CC,\ ".L1: .L2: .nops (.L2 - .L1)$(comma)9",-DHAVE_AS_NOPS_DIRECTIVE) --- a/xen/arch/x86/include/asm/alternative.h +++ b/xen/arch/x86/include/asm/alternative.h @@ -35,19 +35,19 @@ extern void alternative_branches(void); #define alt_repl_e(num)".LXEN%=_repl_e"#num #define alt_repl_len(num) "(" alt_repl_e(num) " - " alt_repl_s(num) ")" -/* GAS's idea of true is -1, while Clang's idea is 1. */ -#ifdef HAVE_AS_NEGATIVE_TRUE -# define AS_TRUE "-" -#else -# define AS_TRUE "" -#endif +/* + * GAS's idea of true is sometimes 1 and sometimes -1, while Clang's idea + * was consistently 1 up to 6.x (it matches GAS's now). Transform it to + * uniformly 1. + */ +#define AS_TRUE(x) "((" x ") & 1)" -#define as_max(a, b) "(("a") ^ ((("a") ^ ("b")) & -("AS_TRUE"(("a") < ("b")" +#define as_max(a, b) "(("a") ^ ((("a") ^ ("b")) & -("AS_TRUE("("a") < ("b")")")))" #define OLDINSTR(oldinstr, padding) \ ".LXEN%=_orig_s:\n\t" oldinstr "\n .LXEN%=_orig_e:\n\t" \ ".LXEN%=_diff = " padding "\n\t" \ -"mknops ("AS_TRUE"(.LXEN%=_diff > 0) * .LXEN%=_diff)\n\t"\ +"mknops ("AS_TRUE(".LXEN%=_diff > 0")" * .LXEN%=_diff)\n\t" \ ".LXEN%=_orig_p:\n\t" #define OLDINSTR_1(oldinstr, n1) \ --- a/xen/arch/x86/include/asm/alternative-asm.h +++ b/xen/arch/x86/include/asm/alternative-asm.h @@ -29,12 +29,12 @@ #endif .endm -/* GAS's idea of true is -1, while Clang's idea is 1. */ -#ifdef HAVE_AS_NEGATIVE_TRUE -# define as_true(x) (-(x)) -#else -# define as_true(x) (x) -#endif +/* + * GAS's idea of true is sometimes 1 and sometimes -1, while Clang's idea + * was consistently 1 up to 6.x (it matches GAS's now). Transform it to + * uniformly 1. + */ +#define as_true(x) ((x) & 1) #define decl_orig(insn, padding) \ .L\@_orig_s: insn; .L\@_orig_e: \
Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature
> On 23 May 2023, at 12:53, Jan Beulich wrote: > > On 23.05.2023 13:50, Luca Fancellu wrote: >>> On 23 May 2023, at 11:31, Jan Beulich wrote: >>> On 23.05.2023 12:21, Luca Fancellu wrote: > On 23 May 2023, at 11:02, Jan Beulich wrote: > On 23.05.2023 09:43, Luca Fancellu wrote: >> @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems. >> >> If using this option is necessary to fix an issue, please report a bug. >> >> +Enables features on dom0 on Arm systems. >> + >> +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain >> and sets >> +the maximum SVE vector length, the option is applicable only to >> AArch64 >> +guests. > > Why "guests"? Does the option affect more than Dom0? I used “guests” because in my mind I was referring to all the aarch64 OS that can be used as control domain, I can change it if it sounds bad. >>> >>> If you means OSes then better also say OSes. But maybe this doesn't need >>> specifically expressing, by saying e.g. "..., the option is applicable >>> only on AArch64"? Or can a Dom0 be 32-bit on Arm64 Xen? >> >> I think there is no limitation so Dom0 can be 32 bit or 64. Maybe I can say >> “... AArch64 kernel guests.”? > > I'd recommend to avoid the term "guest" when you talk about Dom0 alone. > Commonly "guest" means ordinary domains only, i.e. in particular excluding > Dom0. What's wrong with "AArch64 Dom0 kernels"? Ok works for me, I will use “AArch64 Dom0 kernels", I thought “guests” were a generic category and then we have “privilegedguests”, for example Dom0 or driver domain, and “unprivileged guests” like DomUs. > >> +A value equal to 0 disables the feature, this is the default value. >> +Values below 0 means the feature uses the maximum SVE vector length >> +supported by hardware, if SVE is supported. >> +Values above 0 explicitly set the maximum SVE vector length for >> Dom0, >> +allowed values are from 128 to maximum 2048, being multiple of 128. >> +Please note that when the user explicitly specifies the value, if >> that value >> +is above the hardware supported maximum SVE vector length, the >> domain >> +creation will fail and the system will stop, the same will occur if >> the >> +option is provided with a non zero value, but the platform doesn't >> support >> +SVE. > > Assuming this also covers the -1 case, I wonder if that isn't a little too > strict. "Maximum supported" imo can very well be 0. Maximum supported, when platforms uses SVE, can be at minimum 128 by arm specs. >>> >>> When there is SVE - sure. But when there's no SVE, 0 is kind of the implied >>> length. And I'd view a command line option value of -1 quite okay in that >>> case: They've asked for the maximum supported, so they'll get 0. No reason >>> to crash the system during boot. >> >> Ok I see what you mean, for example when Kconfig SVE is enabled, but the >> platform doesn’t >> have SVE feature, requesting sve=-1 will keep the value to 0, and no system >> will be stopped. >> >> Maybe I can say: >> >> “... the same will occur if the option is provided with a positive non zero >> value, >> but the platform doesn't support SVE." > > Right, provided that matches the implementation. Ok I will do the changes, can I retain your R-by? I suppose it covers also documentation right? > > Jan
Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature
On 23.05.2023 13:50, Luca Fancellu wrote: >> On 23 May 2023, at 11:31, Jan Beulich wrote: >> On 23.05.2023 12:21, Luca Fancellu wrote: On 23 May 2023, at 11:02, Jan Beulich wrote: On 23.05.2023 09:43, Luca Fancellu wrote: > @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems. > >If using this option is necessary to fix an issue, please report a bug. > > +Enables features on dom0 on Arm systems. > + > +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain > and sets > +the maximum SVE vector length, the option is applicable only to > AArch64 > +guests. Why "guests"? Does the option affect more than Dom0? >>> >>> I used “guests” because in my mind I was referring to all the aarch64 OS >>> that can be used >>> as control domain, I can change it if it sounds bad. >> >> If you means OSes then better also say OSes. But maybe this doesn't need >> specifically expressing, by saying e.g. "..., the option is applicable >> only on AArch64"? Or can a Dom0 be 32-bit on Arm64 Xen? > > I think there is no limitation so Dom0 can be 32 bit or 64. Maybe I can say > “... AArch64 kernel guests.”? I'd recommend to avoid the term "guest" when you talk about Dom0 alone. Commonly "guest" means ordinary domains only, i.e. in particular excluding Dom0. What's wrong with "AArch64 Dom0 kernels"? > +A value equal to 0 disables the feature, this is the default value. > +Values below 0 means the feature uses the maximum SVE vector length > +supported by hardware, if SVE is supported. > +Values above 0 explicitly set the maximum SVE vector length for Dom0, > +allowed values are from 128 to maximum 2048, being multiple of 128. > +Please note that when the user explicitly specifies the value, if > that value > +is above the hardware supported maximum SVE vector length, the domain > +creation will fail and the system will stop, the same will occur if > the > +option is provided with a non zero value, but the platform doesn't > support > +SVE. Assuming this also covers the -1 case, I wonder if that isn't a little too strict. "Maximum supported" imo can very well be 0. >>> >>> Maximum supported, when platforms uses SVE, can be at minimum 128 by arm >>> specs. >> >> When there is SVE - sure. But when there's no SVE, 0 is kind of the implied >> length. And I'd view a command line option value of -1 quite okay in that >> case: They've asked for the maximum supported, so they'll get 0. No reason >> to crash the system during boot. > > Ok I see what you mean, for example when Kconfig SVE is enabled, but the > platform doesn’t > have SVE feature, requesting sve=-1 will keep the value to 0, and no system > will be stopped. > > Maybe I can say: > > “... the same will occur if the option is provided with a positive non zero > value, > but the platform doesn't support SVE." Right, provided that matches the implementation. Jan
Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature
> On 23 May 2023, at 11:31, Jan Beulich wrote: > > On 23.05.2023 12:21, Luca Fancellu wrote: >>> On 23 May 2023, at 11:02, Jan Beulich wrote: >>> On 23.05.2023 09:43, Luca Fancellu wrote: @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems. If using this option is necessary to fix an issue, please report a bug. +Enables features on dom0 on Arm systems. + +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain and sets +the maximum SVE vector length, the option is applicable only to AArch64 +guests. >>> >>> Why "guests"? Does the option affect more than Dom0? >> >> I used “guests” because in my mind I was referring to all the aarch64 OS >> that can be used >> as control domain, I can change it if it sounds bad. > > If you means OSes then better also say OSes. But maybe this doesn't need > specifically expressing, by saying e.g. "..., the option is applicable > only on AArch64"? Or can a Dom0 be 32-bit on Arm64 Xen? I think there is no limitation so Dom0 can be 32 bit or 64. Maybe I can say “... AArch64 kernel guests.”? > +A value equal to 0 disables the feature, this is the default value. +Values below 0 means the feature uses the maximum SVE vector length +supported by hardware, if SVE is supported. +Values above 0 explicitly set the maximum SVE vector length for Dom0, +allowed values are from 128 to maximum 2048, being multiple of 128. +Please note that when the user explicitly specifies the value, if that value +is above the hardware supported maximum SVE vector length, the domain +creation will fail and the system will stop, the same will occur if the +option is provided with a non zero value, but the platform doesn't support +SVE. >>> >>> Assuming this also covers the -1 case, I wonder if that isn't a little too >>> strict. "Maximum supported" imo can very well be 0. >> >> Maximum supported, when platforms uses SVE, can be at minimum 128 by arm >> specs. > > When there is SVE - sure. But when there's no SVE, 0 is kind of the implied > length. And I'd view a command line option value of -1 quite okay in that > case: They've asked for the maximum supported, so they'll get 0. No reason > to crash the system during boot. Ok I see what you mean, for example when Kconfig SVE is enabled, but the platform doesn’t have SVE feature, requesting sve=-1 will keep the value to 0, and no system will be stopped. Maybe I can say: “... the same will occur if the option is provided with a positive non zero value, but the platform doesn't support SVE." > > Jan
[PATCH v2 2/2] x86: also mark assembler globals hidden
Let's have assembler symbols be consistent with C ones. In principle there are (a few) cases where gas can produce smaller code this way, just that for now there's a gas bug causing smaller code to be emitted even when that shouldn't be the case. Signed-off-by: Jan Beulich --- v2: New. --- a/xen/arch/x86/include/asm/asm_defns.h +++ b/xen/arch/x86/include/asm/asm_defns.h @@ -83,7 +83,7 @@ register unsigned long current_stack_poi #define SYM_ALIGN(algn...) .balign algn -#define SYM_L_GLOBAL(name) .globl name +#define SYM_L_GLOBAL(name) .globl name; .hidden name #define SYM_L_WEAK(name) .weak name #define SYM_L_LOCAL(name) /* nothing */ --- a/xen/arch/x86/include/asm/config.h +++ b/xen/arch/x86/include/asm/config.h @@ -45,11 +45,11 @@ #ifdef __ASSEMBLY__ #define ALIGN .align 16,0x90 #define ENTRY(name) \ - .globl name; \ ALIGN;\ - name: + GLOBAL(name) #define GLOBAL(name)\ .globl name; \ + .hidden name; \ name: #endif
[PATCH v2 1/2] x86: annotate entry points with type and size
Recent gas versions generate minimalistic Dwarf debug info for items annotated as functions and having their sizes specified [1]. "Borrow" Arm's END() and (remotely) derive other annotation infrastructure from Linux'es. For switch_to_kernel() and restore_all_guest() so far implicit alignment (from being first in their respective sections) is being made explicit (as in: using FUNC() without 2nd argument). Whereas for {,compat}create_bounce_frame() and autogen_entrypoints[] alignment is newly arranged for. Except for the added alignment padding (including their knock-on effects) no change in generated code/data. Signed-off-by: Jan Beulich [1] https://sourceware.org/git?p=binutils-gdb.git;a=commitdiff;h=591cc9fbbfd6d51131c0f1d4a92e7893edcc7a28 --- v2: Full rework. --- Only two of the assembly files are being converted for now. More could be done right here or as follow-on in separate patches. In principle the framework should be possible to use by other architectures as well. If we want this, the main questions are going to be: - What header file name? (I don't really like Linux'es linkage.h, so I'd prefer e.g. asm-defns.h or asm_defns.h as we already have in x86.) - How much per-arch customization do we want to permit up front (i.e. without knowing how much of it is going to be needed)? Initially I'd expect only the default function alignment (and padding) to require per-arch definitions. Note that the FB-label in autogen_stubs() cannot be converted just yet: Such labels cannot be used with .type. We could further diverge from Linux'es model and avoid setting STT_NOTYPE explicitly (that's the type labels get by default anyway). Note that we can't use ALIGN() (in place of SYM_ALIGN()) as long as we still have ALIGN. --- a/xen/arch/x86/include/asm/asm_defns.h +++ b/xen/arch/x86/include/asm/asm_defns.h @@ -81,6 +81,45 @@ register unsigned long current_stack_poi #ifdef __ASSEMBLY__ +#define SYM_ALIGN(algn...) .balign algn + +#define SYM_L_GLOBAL(name) .globl name +#define SYM_L_WEAK(name) .weak name +#define SYM_L_LOCAL(name) /* nothing */ + +#define SYM_T_FUNC STT_FUNC +#define SYM_T_DATA STT_OBJECT +#define SYM_T_NONE STT_NOTYPE + +#define SYM(name, typ, linkage, algn...) \ +.type name, SYM_T_ ## typ;\ +SYM_L_ ## linkage(name); \ +SYM_ALIGN(algn); \ +name: + +#define END(name) .size name, . - name + +#define ARG1_(x, y...) (x) +#define ARG2_(x, y...) ARG1_(y) + +#define LAST__(nr) ARG ## nr ## _ +#define LAST_(nr) LAST__(nr) +#define LAST(x, y...) LAST_(count_args(x, ## y))(x, ## y) + +#define FUNC(name, algn...) \ +SYM(name, FUNC, GLOBAL, LAST(16, ## algn), 0x90) +#define LABEL(name, algn...) \ +SYM(name, NONE, GLOBAL, LAST(16, ## algn), 0x90) +#define DATA(name, algn...) \ +SYM(name, DATA, GLOBAL, LAST(0, ## algn), 0xff) + +#define FUNC_LOCAL(name, algn...) \ +SYM(name, FUNC, LOCAL, LAST(16, ## algn), 0x90) +#define LABEL_LOCAL(name, algn...) \ +SYM(name, NONE, LOCAL, LAST(16, ## algn), 0x90) +#define DATA_LOCAL(name, algn...) \ +SYM(name, DATA, LOCAL, LAST(0, ## algn), 0xff) + #ifdef HAVE_AS_QUOTED_SYM #define SUBSECTION_LBL(tag)\ .ifndef .L.tag;\ --- a/xen/arch/x86/x86_64/compat/entry.S +++ b/xen/arch/x86/x86_64/compat/entry.S @@ -8,10 +8,11 @@ #include #include #include +#include #include #include -ENTRY(entry_int82) +FUNC(entry_int82) ENDBR64 ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP pushq $0 @@ -27,9 +28,10 @@ ENTRY(entry_int82) mov %rsp, %rdi call do_entry_int82 +END(entry_int82) /* %rbx: struct vcpu */ -ENTRY(compat_test_all_events) +FUNC(compat_test_all_events) ASSERT_NOT_IN_ATOMIC cli # tests must not race interrupts /*compat_test_softirqs:*/ @@ -66,24 +68,21 @@ compat_test_guest_events: call compat_create_bounce_frame jmp compat_test_all_events -ALIGN /* %rbx: struct vcpu */ -compat_process_softirqs: +LABEL_LOCAL(compat_process_softirqs) sti call do_softirq jmp compat_test_all_events -ALIGN /* %rbx: struct vcpu, %rdx: struct trap_bounce */ -.Lcompat_process_trapbounce: +LABEL_LOCAL(.Lcompat_process_trapbounce) sti .Lcompat_bounce_exception: call compat_create_bounce_frame jmp compat_test_all_events - ALIGN /* %rbx: struct vcpu */ -compat_process_mce: +LABEL_LOCAL(compat_process_mce) testb $1 << VCPU_TRAP_MCE,VCPU_async_exception_mask(%rbx) jnz .Lcompat_test_guest_nmi sti @@ -97,9 +96,8 @@ compat_process_mce: movb %dl,VCPU_async_exception_mask(%rbx) jmp compat_process_trap - ALIGN /* %rbx: struct vcpu */ -compat_process_nmi: +LABEL_LOCAL(compat_process_nmi)
[PATCH v2 0/2] x86: annotate entry points with type and size
The model introduced in patch 1 is in principle arch-agnostic, hence why I'm including Arm and RISC-V reviewers here as well. 1: annotate entry points with type and size 2: also mark assembler globals hidden Jan
Re: PVH Dom0 related UART failure
On Tue, May 23, 2023 at 08:44:48AM +0200, Jan Beulich wrote: > On 23.05.2023 00:20, Stefano Stabellini wrote: > > On Sat, 20 May 2023, Roger Pau Monné wrote: > >> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c > >> index ec2e978a4e6b..0ff8e940fa8d 100644 > >> --- a/xen/drivers/vpci/header.c > >> +++ b/xen/drivers/vpci/header.c > >> @@ -289,6 +289,13 @@ static int modify_bars(const struct pci_dev *pdev, > >> uint16_t cmd, bool rom_only) > >> */ > >> for_each_pdev ( pdev->domain, tmp ) > >> { > >> +if ( !tmp->vpci ) > >> +{ > >> +printk(XENLOG_G_WARNING "%pp: not handled by vPCI for %pd\n", > >> + >sbdf, pdev->domain); > >> +continue; > >> +} > >> + > >> if ( tmp == pdev ) > >> { > >> /* > >> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c > >> index 652807a4a454..0baef3a8d3a1 100644 > >> --- a/xen/drivers/vpci/vpci.c > >> +++ b/xen/drivers/vpci/vpci.c > >> @@ -72,7 +72,12 @@ int vpci_add_handlers(struct pci_dev *pdev) > >> unsigned int i; > >> int rc = 0; > >> > >> -if ( !has_vpci(pdev->domain) ) > >> +if ( !has_vpci(pdev->domain) || > >> + /* > >> + * Ignore RO and hidden devices, those are in use by Xen and vPCI > >> + * won't work on them. > >> + */ > >> + pci_get_pdev(dom_xen, pdev->sbdf) ) > >> return 0; > >> > >> /* We should not get here twice for the same device. */ > > > > > > Now this patch works! Thank you!! :-) > > > > You can check the full logs here > > https://gitlab.com/xen-project/people/sstabellini/xen/-/jobs/4329259080 > > > > Is the patch ready to be upstreamed aside from the commit message? > > I don't think so. vPCI ought to work on "r/o" devices. Out of curiosity, > have you also tried my (hackish and hence RFC) patch [1]? For r/o devices there should be no need of vPCI handlers, reading the config space of such devices can be done directly. There's some work to be done for hidden devices, as for those dom0 has write access to the config space and thus needs vPCI to be setup properly. The change to modify_bars() in order to handle devices without vpci populated is a bugfix, as it's already possible to have devices assigned to a domain that don't have vpci setup, if the call to vpci_add_handlers() in setup_one_hwdom_device() fails. That one could go in separately of the rest of the work in order to enable support for hidden devices. Roger.
[linux-linus test] 180907: regressions - FAIL
flight 180907 linux-linus real [real] flight 180915 linux-linus real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/180907/ http://logs.test-lab.xenproject.org/osstest/logs/180915/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-credit1 8 xen-boot fail REGR. vs. 180278 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 8 xen-boot fail like 180278 test-armhf-armhf-xl-arndale 8 xen-boot fail like 180278 test-armhf-armhf-libvirt-qcow2 8 xen-bootfail like 180278 test-armhf-armhf-libvirt-raw 8 xen-boot fail like 180278 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180278 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180278 test-armhf-armhf-xl-vhd 8 xen-boot fail like 180278 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180278 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180278 test-armhf-armhf-examine 8 reboot fail like 180278 test-armhf-armhf-xl-multivcpu 8 xen-boot fail like 180278 test-armhf-armhf-xl-credit2 8 xen-boot fail like 180278 test-armhf-armhf-xl-rtds 8 xen-boot fail like 180278 test-armhf-armhf-xl 8 xen-boot fail like 180278 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180278 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass version targeted for testing: linuxae8373a5add4ea39f032563cf12a02946d1e3546 baseline version: linux6c538e1adbfc696ac4747fb10d63e704344f763d Last test of basis 180278 2023-04-16 19:41:46 Z 36 days Failing since180281 2023-04-17 06:24:36 Z 36 days 67 attempts Testing same since 180907 2023-05-23 01:11:53 Z0 days1 attempts 2480 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-xl pass
RE: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map
Hi Jan, > -Original Message- > On 23.05.2023 09:32, Henry Wang wrote: > > By looking into the existing implementation of NUMA for DT, > > in Linux, from drivers/base/arch_numa.c: numa_set_distance(), there is a > > "if ((u8)distance != distance)" then return. So I think at least in Linux > > the > > distance value is consistent with the ACPI spec. > > Can we simply gain a similar check (suitably commented), eliminating the > need for saturation? Yes, I will use the similar check and add the related comments in v5. Thanks for your confirmation! Kind regards, Henry > > Jan
Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature
On 23.05.2023 12:21, Luca Fancellu wrote: >> On 23 May 2023, at 11:02, Jan Beulich wrote: >> On 23.05.2023 09:43, Luca Fancellu wrote: >>> @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems. >>> >>> If using this option is necessary to fix an issue, please report a bug. >>> >>> +Enables features on dom0 on Arm systems. >>> + >>> +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain and >>> sets >>> +the maximum SVE vector length, the option is applicable only to AArch64 >>> +guests. >> >> Why "guests"? Does the option affect more than Dom0? > > I used “guests” because in my mind I was referring to all the aarch64 OS that > can be used > as control domain, I can change it if it sounds bad. If you means OSes then better also say OSes. But maybe this doesn't need specifically expressing, by saying e.g. "..., the option is applicable only on AArch64"? Or can a Dom0 be 32-bit on Arm64 Xen? >>> +A value equal to 0 disables the feature, this is the default value. >>> +Values below 0 means the feature uses the maximum SVE vector length >>> +supported by hardware, if SVE is supported. >>> +Values above 0 explicitly set the maximum SVE vector length for Dom0, >>> +allowed values are from 128 to maximum 2048, being multiple of 128. >>> +Please note that when the user explicitly specifies the value, if that >>> value >>> +is above the hardware supported maximum SVE vector length, the domain >>> +creation will fail and the system will stop, the same will occur if the >>> +option is provided with a non zero value, but the platform doesn't >>> support >>> +SVE. >> >> Assuming this also covers the -1 case, I wonder if that isn't a little too >> strict. "Maximum supported" imo can very well be 0. > > Maximum supported, when platforms uses SVE, can be at minimum 128 by arm > specs. When there is SVE - sure. But when there's no SVE, 0 is kind of the implied length. And I'd view a command line option value of -1 quite okay in that case: They've asked for the maximum supported, so they'll get 0. No reason to crash the system during boot. Jan
Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature
> On 23 May 2023, at 11:02, Jan Beulich wrote: > > On 23.05.2023 09:43, Luca Fancellu wrote: >> Add a command line parameter to allow Dom0 the use of SVE resources, >> the command line parameter sve=, sub argument of dom0=, >> controls the feature on this domain and sets the maximum SVE vector >> length for Dom0. >> >> Add a new function, parse_signed_integer(), to parse an integer >> command line argument. >> >> Signed-off-by: Luca Fancellu > > Reviewed-by: Jan Beulich # !arm > >> --- a/docs/misc/xen-command-line.pandoc >> +++ b/docs/misc/xen-command-line.pandoc >> @@ -777,9 +777,9 @@ Specify the bit width of the DMA heap. >> >> ### dom0 >> = List of [ pv | pvh, shadow=, verbose=, >> -cpuid-faulting=, msr-relaxed= ] >> +cpuid-faulting=, msr-relaxed= ] (x86) >> >> -Applicability: x86 >> += List of [ sve= ] (Arm) > > While in the text below you mention this is Arm64 only, I think the tag > here would better express this as well. Ok I can use Arm64 instead if there is no opposition from others > >> @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems. >> >> If using this option is necessary to fix an issue, please report a bug. >> >> +Enables features on dom0 on Arm systems. >> + >> +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain and >> sets >> +the maximum SVE vector length, the option is applicable only to AArch64 >> +guests. > > Why "guests"? Does the option affect more than Dom0? I used “guests” because in my mind I was referring to all the aarch64 OS that can be used as control domain, I can change it if it sounds bad. > >> +A value equal to 0 disables the feature, this is the default value. >> +Values below 0 means the feature uses the maximum SVE vector length >> +supported by hardware, if SVE is supported. >> +Values above 0 explicitly set the maximum SVE vector length for Dom0, >> +allowed values are from 128 to maximum 2048, being multiple of 128. >> +Please note that when the user explicitly specifies the value, if that >> value >> +is above the hardware supported maximum SVE vector length, the domain >> +creation will fail and the system will stop, the same will occur if the >> +option is provided with a non zero value, but the platform doesn't >> support >> +SVE. > > Assuming this also covers the -1 case, I wonder if that isn't a little too > strict. "Maximum supported" imo can very well be 0. Maximum supported, when platforms uses SVE, can be at minimum 128 by arm specs. > > Jan
Re: [patch V4 33/37] cpu/hotplug: Allow "parallel" bringup up to CPUHP_BP_KICK_AP_STATE
On Tue, May 23, 2023 at 01:12:26AM +0200, Thomas Gleixner wrote: > Let me find a brown paperbag and go to sleep before I even try to > compile the obvious fix. That fixes the problem on TX2 - thanks! Tested-by: Mark Brown signature.asc Description: PGP signature
[qemu-mainline test] 180912: regressions - FAIL
flight 180912 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/180912/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 6 xen-buildfail REGR. vs. 180691 build-amd64-xsm 6 xen-buildfail REGR. vs. 180691 build-i386-xsm6 xen-buildfail REGR. vs. 180691 build-i3866 xen-buildfail REGR. vs. 180691 build-arm64 6 xen-buildfail REGR. vs. 180691 build-arm64-xsm 6 xen-buildfail REGR. vs. 180691 build-armhf 6 xen-buildfail REGR. vs. 180691 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 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-shadow 1 build-check(1) blocked n/a test-amd64-i386-xl-vhd1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit1 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-thunderx 1 build-check(1) blocked n/a test-arm64-arm64-xl-vhd 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-coresched-i386-xl 1 build-check(1) blocked n/a test-amd64-coresched-amd64-xl 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-shadow1 build-check(1) blocked n/a test-amd64-amd64-xl-rtds 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-amd64-dom0pvh-xl-amd 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-amd64-dom0pvh-xl-intel 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a
Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map
On 23.05.2023 09:32, Henry Wang wrote: >> -Original Message- >> Subject: Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device >> tree NUMA distance map >> >> The [2] link you provided discusses NUMA_LOCAL_DISTANCE. > > I inferred the discussion as "we should try to keep consistent between the value > used in device tree and ACPI tables". Maybe my inference is wrong. > >> Looking at >> Linux'es Documentation/devicetree/numa.txt, there's no mention of an >> upper bound on the distance values. It only says that on the diagonal >> entries should be 10 (i.e. matching ACPI, without really saying so). > > I agree that the NUMA device tree binding is a little bit vague. So I > cannot > say the case that you provided is not valid. I would like to ask Arm maintainers > (putting them into To:) opinion on this as I think I am not the one to >> decide the > expected behavior on Arm. > > Bertrand/Julien/Stefano: Would you please kindly share your opinion on which > value should be used as the default value of the node distance map? Do you > think reusing the "unreachable" distance, i.e. 0xFF, as the default node distance > is acceptable here? Thanks! My suggestion would be to, rather than rejecting values >= 0xff, to saturate at 0xfe, while keeping 0xff for NUMA_NO_DISTANCE (and overall keeping things consistent with ACPI). >>> >>> Since it has been a while and there were no feedback from Arm >> maintainers, >>> I would like to follow your suggestions for v5. However while I was writing >> the >>> code for the "saturation", i.e., adding below snippet in >> numa_set_distance(): >>> ``` >>> if ( distance > NUMA_NO_DISTANCE ) >>> distance = NUMA_MAX_DISTANCE; >>> ``` >>> I noticed an issue which needs your clarification: >>> Currently, the default value of the distance map is NUMA_NO_DISTANCE, >>> which indicates the nodes are not reachable. IMHO, if in device tree, we do >>> saturations like above for ACPI invalid distances (distances >= 0xff), by >> saturating >>> the distance to 0xfe, we are making the unreachable nodes to reachable. I >> am >>> not sure if this will lead to problems. Do you have any more thoughts? >> Thanks! >> >> I can only answer this with a question back: How is "unreachable" >> represented >> in DT? > > Exactly, that is also what we I am trying to find but failed. My understanding > is that, the spec of NUMA is defined in the ACPI, and the DT NUMA binding > only specifies how users can use DT to represent the same set of ACPI data, > instead of define another standard. > > By looking into the existing implementation of NUMA for DT, > in Linux, from drivers/base/arch_numa.c: numa_set_distance(), there is a > "if ((u8)distance != distance)" then return. So I think at least in Linux the > distance value is consistent with the ACPI spec. Can we simply gain a similar check (suitably commented), eliminating the need for saturation? Jan >> Or is "unreachable" simply expressed by the absence of any data? > > Maybe I am wrong but I don't think so, as in the Linux implementation, > drivers/of/of_numa.c: of_numa_parse_distance_map_v1(), the for loop > "for (i = 0; i + 2 < entry_count; i += 3)" basically implies no fields should > be omitted in the distance map entry. > > Kind regards, > Henry
Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature
On 23.05.2023 09:43, Luca Fancellu wrote: > Add a command line parameter to allow Dom0 the use of SVE resources, > the command line parameter sve=, sub argument of dom0=, > controls the feature on this domain and sets the maximum SVE vector > length for Dom0. > > Add a new function, parse_signed_integer(), to parse an integer > command line argument. > > Signed-off-by: Luca Fancellu Reviewed-by: Jan Beulich # !arm > --- a/docs/misc/xen-command-line.pandoc > +++ b/docs/misc/xen-command-line.pandoc > @@ -777,9 +777,9 @@ Specify the bit width of the DMA heap. > > ### dom0 > = List of [ pv | pvh, shadow=, verbose=, > -cpuid-faulting=, msr-relaxed= ] > +cpuid-faulting=, msr-relaxed= ] (x86) > > -Applicability: x86 > += List of [ sve= ] (Arm) While in the text below you mention this is Arm64 only, I think the tag here would better express this as well. > @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems. > > If using this option is necessary to fix an issue, please report a bug. > > +Enables features on dom0 on Arm systems. > + > +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain and > sets > +the maximum SVE vector length, the option is applicable only to AArch64 > +guests. Why "guests"? Does the option affect more than Dom0? > +A value equal to 0 disables the feature, this is the default value. > +Values below 0 means the feature uses the maximum SVE vector length > +supported by hardware, if SVE is supported. > +Values above 0 explicitly set the maximum SVE vector length for Dom0, > +allowed values are from 128 to maximum 2048, being multiple of 128. > +Please note that when the user explicitly specifies the value, if that > value > +is above the hardware supported maximum SVE vector length, the domain > +creation will fail and the system will stop, the same will occur if the > +option is provided with a non zero value, but the platform doesn't > support > +SVE. Assuming this also covers the -1 case, I wonder if that isn't a little too strict. "Maximum supported" imo can very well be 0. Jan
[PATCH v7 12/12] xen/changelog: Add SVE and "dom0" options to the changelog for Arm
Arm now can use the "dom0=" Xen command line option and the support for guests running SVE instructions is added, put entries in the changelog. Mention the "Tech Preview" status and add an entry in SUPPORT.md Signed-off-by: Luca Fancellu Acked-by: Henry Wang # CHANGELOG --- Changes from v6: - Add Henry's A-by to CHANGELOG Changes from v5: - Add Tech Preview status and add entry in SUPPORT.md (Bertrand) Changes from v4: - No changes Change from v3: - new patch --- CHANGELOG.md | 3 +++ SUPPORT.md | 6 ++ 2 files changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bfd3aa5c0d5..512b7bdc0fcb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) cap toolstack provided values. - Ignore VCPUOP_set_singleshot_timer's VCPU_SSHOTTMR_future flag. The only known user doesn't use it properly, leading to in-guest breakage. + - The "dom0" option is now supported on Arm and "sve=" sub-option can be used + to enable dom0 guest to use SVE/SVE2 instructions. ### Added - On x86, support for features new in Intel Sapphire Rapids CPUs: @@ -20,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Bus-lock detection, used by Xen to mitigate (by rate-limiting) the system wide impact of a guest misusing atomic instructions. - xl/libxl can customize SMBIOS strings for HVM guests. + - On Arm, Xen supports guests running SVE/SVE2 instructions. (Tech Preview) ## [4.17.0](https://xenbits.xen.org/gitweb/?p=xen.git;a=shortlog;h=RELEASE-4.17.0) - 2022-12-12 diff --git a/SUPPORT.md b/SUPPORT.md index 6dbed9d5d029..e0fa2246807b 100644 --- a/SUPPORT.md +++ b/SUPPORT.md @@ -99,6 +99,12 @@ Extension to the GICv3 interrupt controller to support MSI. Status: Experimental +### ARM Scalable Vector Extension (SVE/SVE2) + +AArch64 guest can use Scalable Vector Extension (SVE/SVE2). + +Status: Tech Preview + ## Guest Type ### x86/PV -- 2.34.1
[PATCH v7 11/12] xen/arm: add sve property for dom0less domUs
Add a device tree property in the dom0less domU configuration to enable the guest to use SVE. Update documentation. Signed-off-by: Luca Fancellu --- Changes from v6: - Use ifdef in create_domUs and fail if 'sve' is used on systems with CONFIG_ARM64_SVE not selected (Bertrand, Julien, Jan) Changes from v5: - Stop the domain creation if SVE not supported or SVE VL errors (Julien, Bertrand) - now sve_sanitize_vl_param is renamed to sve_domctl_vl_param and returns a boolean, change the affected code. - Reworded documentation. Changes from v4: - Now it is possible to specify the property "sve" for dom0less device tree node without any value, that means the platform supported VL will be used. Changes from v3: - Now domainconfig_encode_vl is named sve_encode_vl Changes from v2: - xen_domctl_createdomain field name has changed into sve_vl and its value is the VL/128, use domainconfig_encode_vl to encode a plain VL in bits. Changes from v1: - No changes Changes from RFC: - Changed documentation --- docs/misc/arm/device-tree/booting.txt | 16 +++ xen/arch/arm/domain_build.c | 28 +++ 2 files changed, 44 insertions(+) diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt index 3879340b5e0a..32a0e508c471 100644 --- a/docs/misc/arm/device-tree/booting.txt +++ b/docs/misc/arm/device-tree/booting.txt @@ -193,6 +193,22 @@ with the following properties: Optional. Handle to a xen,cpupool device tree node that identifies the cpupool where the guest will be started at boot. +- sve + +Optional. The `sve` property enables Arm SVE usage for the domain and sets +the maximum SVE vector length, the option is applicable only to AArch64 +guests. +A value equal to 0 disables the feature, this is the default value. +Specifying this property with no value, means that the SVE vector length +will be set equal to the maximum vector length supported by the platform. +Values above 0 explicitly set the maximum SVE vector length for the domain, +allowed values are from 128 to maximum 2048, being multiple of 128. +Please note that when the user explicitly specifies the value, if that value +is above the hardware supported maximum SVE vector length, the domain +creation will fail and the system will stop, the same will occur if the +option is provided with a non zero value, but the platform doesn't support +SVE. + - xen,enhanced A string property. Possible property values are: diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 9202a96d9c28..ba4fe9e165ee 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -4008,6 +4008,34 @@ void __init create_domUs(void) d_cfg.max_maptrack_frames = val; } +if ( dt_get_property(node, "sve", ) ) +{ +#ifdef CONFIG_ARM64_SVE +unsigned int sve_vl_bits; +bool ret = false; + +if ( !val ) +{ +/* Property found with no value, means max HW VL supported */ +ret = sve_domctl_vl_param(-1, _vl_bits); +} +else +{ +if ( dt_property_read_u32(node, "sve", ) ) +ret = sve_domctl_vl_param(val, _vl_bits); +else +panic("Error reading 'sve' property"); +} + +if ( ret ) +d_cfg.arch.sve_vl = sve_encode_vl(sve_vl_bits); +else +panic("SVE vector length error\n"); +#else +panic("'sve' property found, but CONFIG_ARM64_SVE not selected"); +#endif +} + /* * The variable max_init_domid is initialized with zero, so here it's * very important to use the pre-increment operator to call -- 2.34.1
[PATCH v7 09/12] tools: add physinfo arch_capabilities handling for Arm
On Arm, the SVE vector length is encoded in arch_capabilities field of struct xen_sysctl_physinfo, make use of this field in the tools when building for arm. Create header arm-arch-capabilities.h to handle the arch_capabilities field of physinfo for Arm. Removed include for xen-tools/common-macros.h in python/xen/lowlevel/xc/xc.c because it is already included by the arm-arch-capabilities.h header. Signed-off-by: Luca Fancellu Acked-by: George Dunlap Acked-by: Christian Lindig Reviewed-by: Anthony PERARD --- Changes from v6: - Fix licence header in arm-atch-capabilities.h, add R-by (Anthony) Changes from v5: - no changes Changes from v4: - Move arm-arch-capabilities.h into xen-tools/, add LIBXL_HAVE_, fixed python return type to I instead of i. (Anthony) Changes from v3: - add Ack-by for the Golang bits (George) - add Ack-by for the OCaml tools (Christian) - now xen-tools/libs.h is named xen-tools/common-macros.h - changed commit message to explain why the header modification in python/xen/lowlevel/xc/xc.c Changes from v2: - rename arm_arch_capabilities.h in arm-arch-capabilities.h, use MASK_EXTR. - Now arm-arch-capabilities.h needs MASK_EXTR macro, but it is defined in libxl_internal.h, it doesn't feel right to include that header so move MASK_EXTR into xen-tools/libs.h that is also included in libxl_internal.h Changes from v1: - now SVE VL is encoded in arch_capabilities on Arm Changes from RFC: - new patch --- tools/golang/xenlight/helpers.gen.go | 2 ++ tools/golang/xenlight/types.gen.go| 1 + tools/include/libxl.h | 6 .../include/xen-tools/arm-arch-capabilities.h | 28 +++ tools/include/xen-tools/common-macros.h | 2 ++ tools/libs/light/libxl.c | 1 + tools/libs/light/libxl_internal.h | 1 - tools/libs/light/libxl_types.idl | 1 + tools/ocaml/libs/xc/xenctrl.ml| 4 +-- tools/ocaml/libs/xc/xenctrl.mli | 4 +-- tools/ocaml/libs/xc/xenctrl_stubs.c | 8 -- tools/python/xen/lowlevel/xc/xc.c | 8 -- tools/xl/xl_info.c| 8 ++ 13 files changed, 62 insertions(+), 12 deletions(-) create mode 100644 tools/include/xen-tools/arm-arch-capabilities.h diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go index 0a203d22321f..35397be2f9e2 100644 --- a/tools/golang/xenlight/helpers.gen.go +++ b/tools/golang/xenlight/helpers.gen.go @@ -3506,6 +3506,7 @@ x.CapVmtrace = bool(xc.cap_vmtrace) x.CapVpmu = bool(xc.cap_vpmu) x.CapGnttabV1 = bool(xc.cap_gnttab_v1) x.CapGnttabV2 = bool(xc.cap_gnttab_v2) +x.ArchCapabilities = uint32(xc.arch_capabilities) return nil} @@ -3540,6 +3541,7 @@ xc.cap_vmtrace = C.bool(x.CapVmtrace) xc.cap_vpmu = C.bool(x.CapVpmu) xc.cap_gnttab_v1 = C.bool(x.CapGnttabV1) xc.cap_gnttab_v2 = C.bool(x.CapGnttabV2) +xc.arch_capabilities = C.uint32_t(x.ArchCapabilities) return nil } diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go index a7c17699f80e..3d968a496744 100644 --- a/tools/golang/xenlight/types.gen.go +++ b/tools/golang/xenlight/types.gen.go @@ -1079,6 +1079,7 @@ CapVmtrace bool CapVpmu bool CapGnttabV1 bool CapGnttabV2 bool +ArchCapabilities uint32 } type Connectorinfo struct { diff --git a/tools/include/libxl.h b/tools/include/libxl.h index cfa1a191318c..4fa09ff7635a 100644 --- a/tools/include/libxl.h +++ b/tools/include/libxl.h @@ -525,6 +525,12 @@ */ #define LIBXL_HAVE_PHYSINFO_CAP_GNTTAB 1 +/* + * LIBXL_HAVE_PHYSINFO_ARCH_CAPABILITIES indicates that libxl_physinfo has a + * arch_capabilities field. + */ +#define LIBXL_HAVE_PHYSINFO_ARCH_CAPABILITIES 1 + /* * LIBXL_HAVE_MAX_GRANT_VERSION indicates libxl_domain_build_info has a * max_grant_version field for setting the max grant table version per diff --git a/tools/include/xen-tools/arm-arch-capabilities.h b/tools/include/xen-tools/arm-arch-capabilities.h new file mode 100644 index ..3849e897925d --- /dev/null +++ b/tools/include/xen-tools/arm-arch-capabilities.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: LGPL-2.1-only */ +/* + * Copyright (C) 2023 ARM Ltd. + */ + +#ifndef ARM_ARCH_CAPABILITIES_H +#define ARM_ARCH_CAPABILITIES_H + +#include +#include + +#include + +static inline +unsigned int arch_capabilities_arm_sve(unsigned int arch_capabilities) +{ +#if defined(__aarch64__) +unsigned int sve_vl = MASK_EXTR(arch_capabilities, +XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK); + +/* Vector length is divided by 128 before storing it in arch_capabilities */ +return sve_vl * 128U; +#else +return 0; +#endif +} + +#endif /* ARM_ARCH_CAPABILITIES_H */ diff --git a/tools/include/xen-tools/common-macros.h b/tools/include/xen-tools/common-macros.h index 76b55bf62085..d53b88182560 100644 --- a/tools/include/xen-tools/common-macros.h +++
[PATCH v7 10/12] xen/tools: add sve parameter in XL configuration
Add sve parameter in XL configuration to allow guests to use SVE feature. Signed-off-by: Luca Fancellu --- Changes from v6: - Add check for sve_vl be multiple of 128 (Anthony) Changes from v5: - Update documentation - re-generated golang files Changes from v4: - Rename sve field to sve_vl (Anthony), changed type to libxl_sve_type - Sanity check of sve field in libxl instead of xl, update docs (Anthony) - drop Ack-by from George because of the changes in the Golang bits Changes from v3: - no changes Changes from v2: - domain configuration field name has changed to sve_vl, also its value now is VL/128. - Add Ack-by George for the Golang bits Changes from v1: - updated to use arch_capabilities field for vector length Changes from RFC: - changed libxl_types.idl sve field to uint16 - now toolstack uses info from physinfo to check against the sve XL value - Changed documentation --- docs/man/xl.cfg.5.pod.in | 16 ++ tools/golang/xenlight/helpers.gen.go | 2 ++ tools/golang/xenlight/types.gen.go | 23 +++ tools/include/libxl.h| 5 + tools/libs/light/libxl_arm.c | 33 tools/libs/light/libxl_types.idl | 22 +++ tools/xl/xl_parse.c | 8 +++ 7 files changed, 109 insertions(+) diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 24ac92718288..1b4e13ab647b 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -2955,6 +2955,22 @@ Currently, only the "sbsa_uart" model is supported for ARM. =back +=item B + +The `sve` parameter enables Arm Scalable Vector Extension (SVE) usage for the +guest and sets the maximum SVE vector length, the option is applicable only to +AArch64 guests. +A value equal to "disabled" disables the feature, this is the default value. +Allowed values are "disabled", "128", "256", "384", "512", "640", "768", "896", +"1024", "1152", "1280", "1408", "1536", "1664", "1792", "1920", "2048", "hw". +Specifying "hw" means that the maximum vector length supported by the platform +will be used. +Please be aware that if a specific vector length is passed and its value is +above the maximum vector length supported by the platform, an error will be +raised. + +=back + =head3 x86 =over 4 diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go index 35397be2f9e2..cd1a16e32eac 100644 --- a/tools/golang/xenlight/helpers.gen.go +++ b/tools/golang/xenlight/helpers.gen.go @@ -1149,6 +1149,7 @@ default: return fmt.Errorf("invalid union key '%v'", x.Type)} x.ArchArm.GicVersion = GicVersion(xc.arch_arm.gic_version) x.ArchArm.Vuart = VuartType(xc.arch_arm.vuart) +x.ArchArm.SveVl = SveType(xc.arch_arm.sve_vl) if err := x.ArchX86.MsrRelaxed.fromC(_x86.msr_relaxed);err != nil { return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err) } @@ -1653,6 +1654,7 @@ default: return fmt.Errorf("invalid union key '%v'", x.Type)} xc.arch_arm.gic_version = C.libxl_gic_version(x.ArchArm.GicVersion) xc.arch_arm.vuart = C.libxl_vuart_type(x.ArchArm.Vuart) +xc.arch_arm.sve_vl = C.libxl_sve_type(x.ArchArm.SveVl) if err := x.ArchX86.MsrRelaxed.toC(_x86.msr_relaxed); err != nil { return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err) } diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go index 3d968a496744..b131a7eedc9d 100644 --- a/tools/golang/xenlight/types.gen.go +++ b/tools/golang/xenlight/types.gen.go @@ -490,6 +490,28 @@ TeeTypeNone TeeType = 0 TeeTypeOptee TeeType = 1 ) +type SveType int +const( +SveTypeHw SveType = -1 +SveTypeDisabled SveType = 0 +SveType128 SveType = 128 +SveType256 SveType = 256 +SveType384 SveType = 384 +SveType512 SveType = 512 +SveType640 SveType = 640 +SveType768 SveType = 768 +SveType896 SveType = 896 +SveType1024 SveType = 1024 +SveType1152 SveType = 1152 +SveType1280 SveType = 1280 +SveType1408 SveType = 1408 +SveType1536 SveType = 1536 +SveType1664 SveType = 1664 +SveType1792 SveType = 1792 +SveType1920 SveType = 1920 +SveType2048 SveType = 2048 +) + type RdmReserve struct { Strategy RdmReserveStrategy Policy RdmReservePolicy @@ -564,6 +586,7 @@ TypeUnion DomainBuildInfoTypeUnion ArchArm struct { GicVersion GicVersion Vuart VuartType +SveVl SveType } ArchX86 struct { MsrRelaxed Defbool diff --git a/tools/include/libxl.h b/tools/include/libxl.h index 4fa09ff7635a..cac641a7eba2 100644 --- a/tools/include/libxl.h +++ b/tools/include/libxl.h @@ -283,6 +283,11 @@ */ #define LIBXL_HAVE_BUILDINFO_ARCH_ARM_TEE 1 +/* + * libxl_domain_build_info has the arch_arm.sve_vl field. + */ +#define LIBXL_HAVE_BUILDINFO_ARCH_ARM_SVE_VL 1 + /* * LIBXL_HAVE_SOFT_RESET indicates that libxl supports performing * 'soft reset' for domains and there is 'soft_reset' shutdown reason diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c index 97c80d7ed0fa..35f76dfc21e4 100644 ---
[PATCH v7 06/12] xen/common: add dom0 xen command line argument for Arm
Currently x86 defines a Xen command line argument dom0= where there can be specified dom0 controlling sub-options, to use it also on Arm, move the code that loops through the list of arguments from x86 to the common code and from there, call architecture specific functions to handle the comma separated sub-options. No functional changes are intended. Signed-off-by: Luca Fancellu Reviewed-by: Jan Beulich Reviewed-by: Bertrand Marquis --- Changes from v6: - no changes Changes from v5: - Add Bertrand R-by Changes from v4: - return EINVAL in Arm implementation of parse_arch_dom0_param, shorten variable name in the funtion from str_begin, str_end to s, e. Removed variable rc from x86 parse_arch_dom0_param implementation. (Jan) - Add R-By Jan Changes from v3: - new patch --- xen/arch/arm/domain_build.c | 5 xen/arch/x86/dom0_build.c | 48 ++--- xen/common/domain.c | 23 ++ xen/include/xen/domain.h| 1 + 4 files changed, 47 insertions(+), 30 deletions(-) diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 9dd1ed5bce44..f373a5024783 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -60,6 +60,11 @@ static int __init parse_dom0_mem(const char *s) } custom_param("dom0_mem", parse_dom0_mem); +int __init parse_arch_dom0_param(const char *s, const char *e) +{ +return -EINVAL; +} + /* Override macros from asm/page.h to make them work with mfn_t */ #undef virt_to_mfn #define virt_to_mfn(va) _mfn(__virt_to_mfn(va)) diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c index 79234f18ff01..9f5300a3efbb 100644 --- a/xen/arch/x86/dom0_build.c +++ b/xen/arch/x86/dom0_build.c @@ -266,42 +266,30 @@ bool __initdata opt_dom0_pvh = !IS_ENABLED(CONFIG_PV); bool __initdata opt_dom0_verbose = IS_ENABLED(CONFIG_VERBOSE_DEBUG); bool __initdata opt_dom0_msr_relaxed; -static int __init cf_check parse_dom0_param(const char *s) +int __init parse_arch_dom0_param(const char *s, const char *e) { -const char *ss; -int rc = 0; +int val; -do { -int val; - -ss = strchr(s, ','); -if ( !ss ) -ss = strchr(s, '\0'); - -if ( IS_ENABLED(CONFIG_PV) && !cmdline_strcmp(s, "pv") ) -opt_dom0_pvh = false; -else if ( IS_ENABLED(CONFIG_HVM) && !cmdline_strcmp(s, "pvh") ) -opt_dom0_pvh = true; +if ( IS_ENABLED(CONFIG_PV) && !cmdline_strcmp(s, "pv") ) +opt_dom0_pvh = false; +else if ( IS_ENABLED(CONFIG_HVM) && !cmdline_strcmp(s, "pvh") ) +opt_dom0_pvh = true; #ifdef CONFIG_SHADOW_PAGING -else if ( (val = parse_boolean("shadow", s, ss)) >= 0 ) -opt_dom0_shadow = val; +else if ( (val = parse_boolean("shadow", s, e)) >= 0 ) +opt_dom0_shadow = val; #endif -else if ( (val = parse_boolean("verbose", s, ss)) >= 0 ) -opt_dom0_verbose = val; -else if ( IS_ENABLED(CONFIG_PV) && - (val = parse_boolean("cpuid-faulting", s, ss)) >= 0 ) -opt_dom0_cpuid_faulting = val; -else if ( (val = parse_boolean("msr-relaxed", s, ss)) >= 0 ) -opt_dom0_msr_relaxed = val; -else -rc = -EINVAL; - -s = ss + 1; -} while ( *ss ); +else if ( (val = parse_boolean("verbose", s, e)) >= 0 ) +opt_dom0_verbose = val; +else if ( IS_ENABLED(CONFIG_PV) && + (val = parse_boolean("cpuid-faulting", s, e)) >= 0 ) +opt_dom0_cpuid_faulting = val; +else if ( (val = parse_boolean("msr-relaxed", s, e)) >= 0 ) +opt_dom0_msr_relaxed = val; +else +return -EINVAL; -return rc; +return 0; } -custom_param("dom0", parse_dom0_param); static char __initdata opt_dom0_ioports_disable[200] = ""; string_param("dom0_ioports_disable", opt_dom0_ioports_disable); diff --git a/xen/common/domain.c b/xen/common/domain.c index 6a440590fe2a..caaa40263792 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -364,6 +364,29 @@ static int __init cf_check parse_extra_guest_irqs(const char *s) } custom_param("extra_guest_irqs", parse_extra_guest_irqs); +static int __init cf_check parse_dom0_param(const char *s) +{ +const char *ss; +int rc = 0; + +do { +int ret; + +ss = strchr(s, ','); +if ( !ss ) +ss = strchr(s, '\0'); + +ret = parse_arch_dom0_param(s, ss); +if ( ret && !rc ) +rc = ret; + +s = ss + 1; +} while ( *ss ); + +return rc; +} +custom_param("dom0", parse_dom0_param); + /* * Release resources held by a domain. There may or may not be live * references to the domain, and it may or may not be fully constructed. diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h index 26f9c4f6dd5b..1df8f933d076 100644 --- a/xen/include/xen/domain.h +++ b/xen/include/xen/domain.h @@ -16,6 +16,7 @@ typedef union { struct vcpu
[PATCH v7 04/12] xen/arm: add SVE exception class handling
SVE has a new exception class with code 0x19, introduce the new code and handle the exception. Signed-off-by: Luca Fancellu Reviewed-by: Bertrand Marquis Reviewed-by: Julien Grall --- Changes from v6: - Add R-by Julien Changes from v5: - modified error messages (Julien) - add R-by Bertrand Changes from v4: - No changes Changes from v3: - No changes Changes from v2: - No changes Changes from v1: - No changes Changes from RFC: - No changes --- xen/arch/arm/include/asm/processor.h | 1 + xen/arch/arm/traps.c | 9 + 2 files changed, 10 insertions(+) diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h index bc683334125c..7e42ff8811fc 100644 --- a/xen/arch/arm/include/asm/processor.h +++ b/xen/arch/arm/include/asm/processor.h @@ -426,6 +426,7 @@ #define HSR_EC_HVC640x16 #define HSR_EC_SMC640x17 #define HSR_EC_SYSREG 0x18 +#define HSR_EC_SVE 0x19 #endif #define HSR_EC_INSTR_ABORT_LOWER_EL 0x20 #define HSR_EC_INSTR_ABORT_CURR_EL 0x21 diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 3393e10b52e6..f6437f6aa9c9 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -2172,6 +2172,11 @@ void do_trap_guest_sync(struct cpu_user_regs *regs) perfc_incr(trap_sysreg); do_sysreg(regs, hsr); break; +case HSR_EC_SVE: +GUEST_BUG_ON(regs_mode_is_32bit(regs)); +gprintk(XENLOG_WARNING, "Domain tried to use SVE while not allowed\n"); +inject_undef_exception(regs, hsr); +break; #endif case HSR_EC_INSTR_ABORT_LOWER_EL: @@ -2201,6 +2206,10 @@ void do_trap_hyp_sync(struct cpu_user_regs *regs) case HSR_EC_BRK: do_trap_brk(regs, hsr); break; +case HSR_EC_SVE: +/* An SVE exception is a bug somewhere in hypervisor code */ +do_unexpected_trap("SVE trap at EL2", regs); +break; #endif case HSR_EC_DATA_ABORT_CURR_EL: case HSR_EC_INSTR_ABORT_CURR_EL: -- 2.34.1
[PATCH v7 05/12] arm/sve: save/restore SVE context switch
Save/restore context switch for SVE, allocate memory to contain the Z0-31 registers whose length is maximum 2048 bits each and FFR who can be maximum 256 bits, the allocated memory depends on how many bits is the vector length for the domain and how many bits are supported by the platform. Save P0-15 whose length is maximum 256 bits each, in this case the memory used is from the fpregs field in struct vfp_state, because V0-31 are part of Z0-31 and this space would have been unused for SVE domain otherwise. Create zcr_el{1,2} fields in arch_vcpu, initialise zcr_el2 on vcpu creation given the requested vector length and restore it on context switch, save/restore ZCR_EL1 value as well. List import macros from Linux in README.LinuxPrimitives. Signed-off-by: Luca Fancellu --- Changes from v6: - Add comment for explain why sve_save/sve_load are different from Linux, add macros in xen/arch/arm/README.LinuxPrimitives (Julien) - Add comments in sve_context_init and sve_context_free, handle the case where sve_zreg_ctx_end is NULL, move setting of v->arch.zcr_el2 in sve_context_init (Julien) - remove stubs for sve_context_* and sve_save_* and rely on compiler DCE (Jan) - Add comments for sve_save_ctx/sve_load_ctx (Julien) Changes from v5: - use XFREE instead of xfree, keep the headers (Julien) - Avoid math computation for every save/restore, store the computation in struct vfp_state once (Bertrand) - protect access to v->domain->arch.sve_vl inside arch_vcpu_create now that sve_vl is available only on arm64 Changes from v4: - No changes Changes from v3: - don't use fixed len types when not needed (Jan) - now VL is an encoded value, decode it before using. Changes from v2: - No changes Changes from v1: - No changes Changes from RFC: - Moved zcr_el2 field introduction in this patch, restore its content inside sve_restore_state function. (Julien) fix patch 5 Signed-off-by: Luca Fancellu Change-Id: Ief65b2ff14fd579afa4fd110ce08a19980e64fa9 --- xen/arch/arm/README.LinuxPrimitives | 4 +- xen/arch/arm/arm64/sve-asm.S | 147 +++ xen/arch/arm/arm64/sve.c | 91 ++ xen/arch/arm/arm64/vfp.c | 79 ++-- xen/arch/arm/domain.c| 6 + xen/arch/arm/include/asm/arm64/sve.h | 4 + xen/arch/arm/include/asm/arm64/sysregs.h | 3 + xen/arch/arm/include/asm/arm64/vfp.h | 12 ++ xen/arch/arm/include/asm/domain.h| 2 + 9 files changed, 313 insertions(+), 35 deletions(-) diff --git a/xen/arch/arm/README.LinuxPrimitives b/xen/arch/arm/README.LinuxPrimitives index 76c8df29e416..301c0271bbe4 100644 --- a/xen/arch/arm/README.LinuxPrimitives +++ b/xen/arch/arm/README.LinuxPrimitives @@ -69,7 +69,9 @@ SVE assembly macro: last sync @ v6.3.0 (last commit: 457391b03803) linux/arch/arm64/include/asm/fpsimdmacros.h xen/arch/arm/include/asm/arm64/sve-asm.S The following macros were taken from Linux: -_check_general_reg, _check_num, _sve_rdvl +_check_general_reg, _check_num, _sve_rdvl, __for, _for, _sve_check_zreg, +_sve_check_preg, _sve_str_v, _sve_ldr_v, _sve_str_p, _sve_ldr_p, _sve_rdffr, +_sve_wrffr = arm32 diff --git a/xen/arch/arm/arm64/sve-asm.S b/xen/arch/arm/arm64/sve-asm.S index 4d1549344733..59dbefbbb252 100644 --- a/xen/arch/arm/arm64/sve-asm.S +++ b/xen/arch/arm/arm64/sve-asm.S @@ -17,6 +17,18 @@ .endif .endm +.macro _sve_check_zreg znr +.if (\znr) < 0 || (\znr) > 31 +.error "Bad Scalable Vector Extension vector register number \znr." +.endif +.endm + +.macro _sve_check_preg pnr +.if (\pnr) < 0 || (\pnr) > 15 +.error "Bad Scalable Vector Extension predicate register number \pnr." +.endif +.endm + .macro _check_num n, min, max .if (\n) < (\min) || (\n) > (\max) .error "Number \n out of range [\min,\max]" @@ -26,6 +38,54 @@ /* SVE instruction encodings for non-SVE-capable assemblers */ /* (pre binutils 2.28, all kernel capable clang versions support SVE) */ +/* STR (vector): STR Z\nz, [X\nxbase, #\offset, MUL VL] */ +.macro _sve_str_v nz, nxbase, offset=0 +_sve_check_zreg \nz +_check_general_reg \nxbase +_check_num (\offset), -0x100, 0xff +.inst 0xe5804000\ +| (\nz) \ +| ((\nxbase) << 5) \ +| (((\offset) & 7) << 10) \ +| (((\offset) & 0x1f8) << 13) +.endm + +/* LDR (vector): LDR Z\nz, [X\nxbase, #\offset, MUL VL] */ +.macro _sve_ldr_v nz, nxbase, offset=0 +_sve_check_zreg \nz +_check_general_reg \nxbase +_check_num (\offset), -0x100, 0xff +.inst 0x85804000\ +| (\nz) \ +| ((\nxbase) << 5) \ +| (((\offset) & 7) << 10) \ +| (((\offset) & 0x1f8) << 13) +.endm + +/* STR (predicate): STR P\np, [X\nxbase, #\offset, MUL VL] */ +.macro
[PATCH v7 08/12] xen/physinfo: encode Arm SVE vector length in arch_capabilities
When the arm platform supports SVE, advertise the feature in the field arch_capabilities in struct xen_sysctl_physinfo by encoding the SVE vector length in it. Signed-off-by: Luca Fancellu Reviewed-by: Bertrand Marquis --- Changes from v6: - no changes Changes from v5: - Add R-by from Bertrand Changes from v4: - Write arch_capabilities from arch_do_physinfo instead of using stub functions (Jan) Changes from v3: - domainconfig_encode_vl is now named sve_encode_vl Changes from v2: - Remove XEN_SYSCTL_PHYSCAP_ARM_SVE_SHFT, use MASK_INSR and protect with ifdef XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK (Jan) - Use the helper function sve_arch_cap_physinfo to encode the VL into physinfo arch_capabilities field. Changes from v1: - Use only arch_capabilities and some defines to encode SVE VL (Bertrand, Stefano, Jan) Changes from RFC: - new patch --- xen/arch/arm/sysctl.c | 4 xen/include/public/sysctl.h | 4 2 files changed, 8 insertions(+) diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c index b0a78a8b10d0..e9a0661146e4 100644 --- a/xen/arch/arm/sysctl.c +++ b/xen/arch/arm/sysctl.c @@ -11,11 +11,15 @@ #include #include #include +#include #include void arch_do_physinfo(struct xen_sysctl_physinfo *pi) { pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm | XEN_SYSCTL_PHYSCAP_hap; + +pi->arch_capabilities |= MASK_INSR(sve_encode_vl(get_sys_vl_len()), + XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK); } long arch_do_sysctl(struct xen_sysctl *sysctl, diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h index 2b24d6bfd00e..9d06e92d0f6a 100644 --- a/xen/include/public/sysctl.h +++ b/xen/include/public/sysctl.h @@ -94,6 +94,10 @@ struct xen_sysctl_tbuf_op { /* Max XEN_SYSCTL_PHYSCAP_* constant. Used for ABI checking. */ #define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_gnttab_v2 +#if defined(__arm__) || defined(__aarch64__) +#define XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK (0x1FU) +#endif + struct xen_sysctl_physinfo { uint32_t threads_per_core; uint32_t cores_per_socket; -- 2.34.1
[PATCH v7 07/12] xen: enable Dom0 to use SVE feature
Add a command line parameter to allow Dom0 the use of SVE resources, the command line parameter sve=, sub argument of dom0=, controls the feature on this domain and sets the maximum SVE vector length for Dom0. Add a new function, parse_signed_integer(), to parse an integer command line argument. Signed-off-by: Luca Fancellu --- Changes from v6: - Fixed case for e==NULL in parse_signed_integer, drop parenthesis from if conditions, delete inline sve_domctl_vl_param and rely on DCE from the compiler (Jan) - Drop parenthesis from opt_dom0_sve (Julien) - Do not continue if 'sve' is in command line args but CONFIG_ARM64_SVE is not selected: https://lore.kernel.org/all/7614ae25-f59d-430a-9c3e-30b1ce0e1...@arm.com/ Changes from v5: - stop the domain if VL error occurs (Julien, Bertrand) - update the documentation - Rename sve_sanitize_vl_param to sve_domctl_vl_param to mark the fact that we are sanitizing a parameter coming from the user before encoding it into sve_vl in domctl structure. (suggestion from Bertrand in a separate discussion) - update comment in parse_signed_integer, return boolean in sve_domctl_vl_param (Jan). Changes from v4: - Negative values as user param means max supported HW VL (Jan) - update documentation, make use of no_config_param(), rename parse_integer into parse_signed_integer and take long long *, also put a comment on the -2 return condition, update declaration comment to reflect the modifications (Jan) Changes from v3: - Don't use fixed len types when not needed (Jan) - renamed domainconfig_encode_vl to sve_encode_vl - Use a sub argument of dom0= to enable the feature (Jan) - Add parse_integer() function Changes from v2: - xen_domctl_createdomain field has changed into sve_vl and its value now is the VL / 128, create an helper function for that. Changes from v1: - No changes Changes from RFC: - Changed docs to explain that the domain won't be created if the requested vector length is above the supported one from the platform. --- docs/misc/xen-command-line.pandoc| 20 ++-- xen/arch/arm/arm64/sve.c | 20 xen/arch/arm/domain_build.c | 26 ++ xen/arch/arm/include/asm/arm64/sve.h | 10 ++ xen/common/kernel.c | 28 xen/include/xen/lib.h| 10 ++ 6 files changed, 112 insertions(+), 2 deletions(-) diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc index e0b89b7d3319..47e5b4eb6199 100644 --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -777,9 +777,9 @@ Specify the bit width of the DMA heap. ### dom0 = List of [ pv | pvh, shadow=, verbose=, -cpuid-faulting=, msr-relaxed= ] +cpuid-faulting=, msr-relaxed= ] (x86) -Applicability: x86 += List of [ sve= ] (Arm) Controls for how dom0 is constructed on x86 systems. @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems. If using this option is necessary to fix an issue, please report a bug. +Enables features on dom0 on Arm systems. + +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain and sets +the maximum SVE vector length, the option is applicable only to AArch64 +guests. +A value equal to 0 disables the feature, this is the default value. +Values below 0 means the feature uses the maximum SVE vector length +supported by hardware, if SVE is supported. +Values above 0 explicitly set the maximum SVE vector length for Dom0, +allowed values are from 128 to maximum 2048, being multiple of 128. +Please note that when the user explicitly specifies the value, if that value +is above the hardware supported maximum SVE vector length, the domain +creation will fail and the system will stop, the same will occur if the +option is provided with a non zero value, but the platform doesn't support +SVE. + ### dom0-cpuid = List of comma separated booleans diff --git a/xen/arch/arm/arm64/sve.c b/xen/arch/arm/arm64/sve.c index 84a6dedc1fd7..feaca2cf647d 100644 --- a/xen/arch/arm/arm64/sve.c +++ b/xen/arch/arm/arm64/sve.c @@ -13,6 +13,9 @@ #include #include +/* opt_dom0_sve: allow Dom0 to use SVE and set maximum vector length. */ +int __initdata opt_dom0_sve; + extern unsigned int sve_get_hw_vl(void); /* @@ -152,6 +155,23 @@ void sve_restore_state(struct vcpu *v) sve_load_ctx(v->arch.vfp.sve_zreg_ctx_end, v->arch.vfp.fpregs, 1); } +bool __init sve_domctl_vl_param(int val, unsigned int *out) +{ +/* + * Negative SVE parameter value means to use the maximum supported + * vector length, otherwise if a positive value is provided, check if the + * vector length is a multiple of 128 + */ +if ( val < 0 ) +*out = get_sys_vl_len(); +else if ( (val % SVE_VL_MULTIPLE_VAL) == 0 ) +
[PATCH v7 00/12] SVE feature for arm guests
This serie is introducing the possibility for Dom0 and DomU guests to use sve/sve2 instructions. SVE feature introduces new instruction and registers to improve performances on floating point operations. The SVE feature is advertised using the ID_AA64PFR0_EL1 register, SVE field, and when available the ID_AA64ZFR0_EL1 register provides additional information about the implemented version and other SVE feature. New registers added by the SVE feature are Z0-Z31, P0-P15, FFR, ZCR_ELx. Z0-Z31 are scalable vector register whose size is implementation defined and goes from 128 bits to maximum 2048, the term vector length will be used to refer to this quantity. P0-P15 are predicate registers and the size is the vector length divided by 8, same size is the FFR (First Fault Register). ZCR_ELx is a register that can control and restrict the maximum vector length used by the exception level and all the lower exception levels, so for example EL3 can restrict the vector length usable by EL3,2,1,0. The platform has a maximum implemented vector length, so for every value written in ZCR register, if this value is above the implemented length, then the lower value will be used. The RDVL instruction can be used to check what vector length is the HW using after setting ZCR. For an SVE guest, the V0-V31 registers are part of the Z0-Z31, so there is no need to save them separately, saving Z0-Z31 will save implicitly also V0-V31. SVE usage can be trapped using a flag in CPTR_EL2, hence in this serie the register is added to the domain state, to be able to trap only the guests that are not allowed to use SVE. This serie is introducing a command line parameter to enable Dom0 to use SVE and to set its maximum vector length that by default is 0 which means the guest is not allowed to use SVE. Values from 128 to 2048 mean the guest can use SVE with the selected value used as maximum allowed vector length (which could be lower if the implemented one is lower). For DomUs, an XL parameter with the same way of use is introduced and a dom0less DTB binding is created. The context switch is the most critical part because there can be big registers to be saved, in this serie an easy approach is used and the context is saved/restored every time for the guests that are allowed to use SVE. Luca Fancellu (12): xen/arm: enable SVE extension for Xen xen/arm: add SVE vector length field to the domain xen/arm: Expose SVE feature to the guest xen/arm: add SVE exception class handling arm/sve: save/restore SVE context switch xen/common: add dom0 xen command line argument for Arm xen: enable Dom0 to use SVE feature xen/physinfo: encode Arm SVE vector length in arch_capabilities tools: add physinfo arch_capabilities handling for Arm xen/tools: add sve parameter in XL configuration xen/arm: add sve property for dom0less domUs xen/changelog: Add SVE and "dom0" options to the changelog for Arm CHANGELOG.md | 3 + SUPPORT.md| 6 + docs/man/xl.cfg.5.pod.in | 16 ++ docs/misc/arm/device-tree/booting.txt | 16 ++ docs/misc/xen-command-line.pandoc | 20 +- tools/golang/xenlight/helpers.gen.go | 4 + tools/golang/xenlight/types.gen.go| 24 +++ tools/include/libxl.h | 11 + .../include/xen-tools/arm-arch-capabilities.h | 28 +++ tools/include/xen-tools/common-macros.h | 2 + tools/libs/light/libxl.c | 1 + tools/libs/light/libxl_arm.c | 33 +++ tools/libs/light/libxl_internal.h | 1 - tools/libs/light/libxl_types.idl | 23 +++ tools/ocaml/libs/xc/xenctrl.ml| 4 +- tools/ocaml/libs/xc/xenctrl.mli | 4 +- tools/ocaml/libs/xc/xenctrl_stubs.c | 8 +- tools/python/xen/lowlevel/xc/xc.c | 8 +- tools/xl/xl_info.c| 8 + tools/xl/xl_parse.c | 8 + xen/arch/arm/Kconfig | 10 +- xen/arch/arm/README.LinuxPrimitives | 11 + xen/arch/arm/arm64/Makefile | 1 + xen/arch/arm/arm64/cpufeature.c | 7 +- xen/arch/arm/arm64/domctl.c | 4 + xen/arch/arm/arm64/sve-asm.S | 195 ++ xen/arch/arm/arm64/sve.c | 182 xen/arch/arm/arm64/vfp.c | 79 --- xen/arch/arm/arm64/vsysreg.c | 41 +++- xen/arch/arm/cpufeature.c | 6 +- xen/arch/arm/domain.c | 55 - xen/arch/arm/domain_build.c | 66 ++ xen/arch/arm/include/asm/arm64/sve.h | 72 +++ xen/arch/arm/include/asm/arm64/sysregs.h | 4 + xen/arch/arm/include/asm/arm64/vfp.h | 12 ++ xen/arch/arm/include/asm/cpufeature.h | 14 ++
[PATCH v7 01/12] xen/arm: enable SVE extension for Xen
Enable Xen to handle the SVE extension, add code in cpufeature module to handle ZCR SVE register, disable trapping SVE feature on system boot only when SVE resources are accessed. While there, correct coding style for the comment on coprocessor trapping. Now cptr_el2 is part of the domain context and it will be restored on context switch, this is a preparation for saving the SVE context which will be part of VFP operations, so restore it before the call to save VFP registers. To save an additional isb barrier, restore cptr_el2 before an existing isb barrier and move the call for saving VFP context after that barrier. To keep a (mostly) specularity of ctxt_switch_to() and ctxt_switch_from(), move vfp_save_state() up in the function. Change the KConfig entry to make ARM64_SVE symbol selectable, by default it will be not selected. Create sve module and sve_asm.S that contains assembly routines for the SVE feature, this code is inspired from linux and it uses instruction encoding to be compatible with compilers that does not support SVE, imported instructions are documented in README.LinuxPrimitives. Signed-off-by: Luca Fancellu --- Changes from v6: - modified licence, add emacs block, move vfp_save_state up in the function, add comments to CPTR_EL2 and vfp_restore_state, don't use variable in init_traps(), code style fixes, add entries to README.LinuxPrimitives (Julien) - vl_to_zcr is moved into sve.c module as changes to the series led to its usage only inside it, remove stub for compute_max_zcr and rely on compiler DCE. Changes from v5: - Add R-by Bertrand Changes from v4: - don't use fixed types in vl_to_zcr, forgot to address that in v3, by mistake I changed that in patch 2, fixing now (Jan) Changes from v3: - no changes Changes from v2: - renamed sve_asm.S in sve-asm.S, new files should not contain underscore in the name (Jan) Changes from v1: - Add assert to vl_to_zcr, it is never called with vl==0, but just to be sure it won't in the future. Changes from RFC: - Moved restoring of cptr before an existing barrier (Julien) - Marked the feature as unsupported for now (Julien) - Trap and un-trap only when using SVE resources in compute_max_zcr() (Julien) --- xen/arch/arm/Kconfig | 10 ++-- xen/arch/arm/README.LinuxPrimitives | 9 xen/arch/arm/arm64/Makefile | 1 + xen/arch/arm/arm64/cpufeature.c | 7 ++- xen/arch/arm/arm64/sve-asm.S | 48 +++ xen/arch/arm/arm64/sve.c | 59 xen/arch/arm/cpufeature.c| 6 ++- xen/arch/arm/domain.c| 20 +--- xen/arch/arm/include/asm/arm64/sve.h | 27 +++ xen/arch/arm/include/asm/arm64/sysregs.h | 1 + xen/arch/arm/include/asm/cpufeature.h| 14 ++ xen/arch/arm/include/asm/domain.h| 1 + xen/arch/arm/include/asm/processor.h | 2 + xen/arch/arm/setup.c | 5 +- xen/arch/arm/traps.c | 27 ++- 15 files changed, 210 insertions(+), 27 deletions(-) create mode 100644 xen/arch/arm/arm64/sve-asm.S create mode 100644 xen/arch/arm/arm64/sve.c create mode 100644 xen/arch/arm/include/asm/arm64/sve.h diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index 239d3aed3c7f..41f45d8d1203 100644 --- a/xen/arch/arm/Kconfig +++ b/xen/arch/arm/Kconfig @@ -112,11 +112,15 @@ config ARM64_PTR_AUTH This feature is not supported in Xen. config ARM64_SVE - def_bool n + bool "Enable Scalar Vector Extension support (UNSUPPORTED)" if UNSUPPORTED depends on ARM_64 help - Scalar Vector Extension support. - This feature is not supported in Xen. + Scalar Vector Extension (SVE/SVE2) support for guests. + + Please be aware that currently, enabling this feature will add latency on + VM context switch between SVE enabled guests, between not-enabled SVE + guests and SVE enabled guests and viceversa, compared to the time + required to switch between not-enabled SVE guests. config ARM64_MTE def_bool n diff --git a/xen/arch/arm/README.LinuxPrimitives b/xen/arch/arm/README.LinuxPrimitives index 1d53e6a898da..76c8df29e416 100644 --- a/xen/arch/arm/README.LinuxPrimitives +++ b/xen/arch/arm/README.LinuxPrimitives @@ -62,6 +62,15 @@ done linux/arch/arm64/lib/clear_page.S xen/arch/arm/arm64/lib/clear_page.S linux/arch/arm64/lib/copy_page.Sunused in Xen +- + +SVE assembly macro: last sync @ v6.3.0 (last commit: 457391b03803) + +linux/arch/arm64/include/asm/fpsimdmacros.h xen/arch/arm/include/asm/arm64/sve-asm.S + +The following macros were taken from Linux: +_check_general_reg, _check_num, _sve_rdvl + = arm32
[PATCH v7 03/12] xen/arm: Expose SVE feature to the guest
When a guest is allowed to use SVE, expose the SVE features through the identification registers. Signed-off-by: Luca Fancellu Acked-by: Julien Grall --- Changes from v6: - code style fix, add A-by Julien Changes from v5: - given the move of is_sve_domain() in asm/arm64/sve.h, add the header to vsysreg.c - dropping Bertrand's R-by because of the change Changes from v4: - no changes Changes from v3: - no changes Changes from v2: - no changes Changes from v1: - No changes Changes from RFC: - No changes --- xen/arch/arm/arm64/vsysreg.c | 41 ++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c index 758750983c11..fe31f7b3827f 100644 --- a/xen/arch/arm/arm64/vsysreg.c +++ b/xen/arch/arm/arm64/vsysreg.c @@ -18,6 +18,8 @@ #include +#include +#include #include #include #include @@ -295,7 +297,28 @@ void do_sysreg(struct cpu_user_regs *regs, GENERATE_TID3_INFO(MVFR0_EL1, mvfr, 0) GENERATE_TID3_INFO(MVFR1_EL1, mvfr, 1) GENERATE_TID3_INFO(MVFR2_EL1, mvfr, 2) -GENERATE_TID3_INFO(ID_AA64PFR0_EL1, pfr64, 0) + +case HSR_SYSREG_ID_AA64PFR0_EL1: +{ +register_t guest_reg_value = guest_cpuinfo.pfr64.bits[0]; + +if ( is_sve_domain(v->domain) ) +{ +/* 4 is the SVE field width in id_aa64pfr0_el1 */ +uint64_t mask = GENMASK(ID_AA64PFR0_SVE_SHIFT + 4 - 1, +ID_AA64PFR0_SVE_SHIFT); +/* sysval is the sve field on the system */ +uint64_t sysval = cpuid_feature_extract_unsigned_field_width( +system_cpuinfo.pfr64.bits[0], +ID_AA64PFR0_SVE_SHIFT, 4); +guest_reg_value &= ~mask; +guest_reg_value |= (sysval << ID_AA64PFR0_SVE_SHIFT) & mask; +} + +return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, 1, + guest_reg_value); +} + GENERATE_TID3_INFO(ID_AA64PFR1_EL1, pfr64, 1) GENERATE_TID3_INFO(ID_AA64DFR0_EL1, dbg64, 0) GENERATE_TID3_INFO(ID_AA64DFR1_EL1, dbg64, 1) @@ -306,7 +329,21 @@ void do_sysreg(struct cpu_user_regs *regs, GENERATE_TID3_INFO(ID_AA64MMFR2_EL1, mm64, 2) GENERATE_TID3_INFO(ID_AA64AFR0_EL1, aux64, 0) GENERATE_TID3_INFO(ID_AA64AFR1_EL1, aux64, 1) -GENERATE_TID3_INFO(ID_AA64ZFR0_EL1, zfr64, 0) + +case HSR_SYSREG_ID_AA64ZFR0_EL1: +{ +/* + * When the guest has the SVE feature enabled, the whole id_aa64zfr0_el1 + * needs to be exposed. + */ +register_t guest_reg_value = guest_cpuinfo.zfr64.bits[0]; + +if ( is_sve_domain(v->domain) ) +guest_reg_value = system_cpuinfo.zfr64.bits[0]; + +return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, 1, + guest_reg_value); +} /* * Those cases are catching all Reserved registers trapped by TID3 which -- 2.34.1
[PATCH v7 02/12] xen/arm: add SVE vector length field to the domain
Add sve_vl field to arch_domain and xen_arch_domainconfig struct, to allow the domain to have an information about the SVE feature and the number of SVE register bits that are allowed for this domain. sve_vl field is the vector length in bits divided by 128, this allows to use less space in the structures. The field is used also to allow or forbid a domain to use SVE, because a value equal to zero means the guest is not allowed to use the feature. Check that the requested vector length is lower or equal to the platform supported vector length, otherwise fail on domain creation. Check that only 64 bit domains have SVE enabled, otherwise fail. Signed-off-by: Luca Fancellu --- Changes from v6: - Style fix, have is_sve_domain as static inline instead of macro (Julien) Changes from v5: - Update commit message stating the interface ver. bump (Bertrand) - in struct arch_domain, protect sve_vl with CONFIG_ARM64_SVE, given the change, move also is_sve_domain() where it's protected inside sve.h and create a stub when the macro is not defined, protect the usage of sve_vl where needed. (Julien) - Add a check for 32 bit guest running on top of 64 bit host that have sve parameter enabled to stop the domain creation, added in construct_domain() of domain_build.c and subarch_do_domctl of domctl.c. (Julien) Changes from v4: - Return 0 in get_sys_vl_len() if sve is not supported, code style fix, removed else if since the conditions can't fallthrough, removed not needed condition checking for VL bits validity because it's already covered, so delete is_vl_valid() function. (Jan) Changes from v3: - don't use fixed types when not needed, use encoded value also in arch_domain so rename sve_vl_bits in sve_vl. (Jan) - rename domainconfig_decode_vl to sve_decode_vl because it will now be used also to decode from arch_domain value - change sve_vl from uint16_t to uint8_t and move it after "type" field to optimize space. Changes from v2: - rename field in xen_arch_domainconfig from "sve_vl_bits" to "sve_vl" and use the implicit padding after gic_version to store it, now this field is the VL/128. (Jan) - Created domainconfig_decode_vl() function to decode the sve_vl field and use it as plain bits value inside arch_domain. - Changed commit message reflecting the changes Changes from v1: - no changes Changes from RFC: - restore zcr_el2 in sve_restore_state, that will be introduced later in this serie, so remove zcr_el2 related code from this patch and move everything to the later patch (Julien) - add explicit padding into struct xen_arch_domainconfig (Julien) - Don't lower down the vector length, just fail to create the domain. (Julien) --- xen/arch/arm/arm64/domctl.c | 4 xen/arch/arm/arm64/sve.c | 12 +++ xen/arch/arm/domain.c| 29 ++ xen/arch/arm/domain_build.c | 7 +++ xen/arch/arm/include/asm/arm64/sve.h | 31 xen/arch/arm/include/asm/domain.h| 5 + xen/include/public/arch-arm.h| 2 ++ 7 files changed, 90 insertions(+) diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c index 0de89b42c448..14fc622e9956 100644 --- a/xen/arch/arm/arm64/domctl.c +++ b/xen/arch/arm/arm64/domctl.c @@ -10,6 +10,7 @@ #include #include #include +#include #include static long switch_mode(struct domain *d, enum domain_type type) @@ -43,6 +44,9 @@ long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d, case 32: if ( !cpu_has_el1_32 ) return -EINVAL; +/* SVE is not supported for 32 bit domain */ +if ( is_sve_domain(d) ) +return -EINVAL; return switch_mode(d, DOMAIN_32BIT); case 64: return switch_mode(d, DOMAIN_64BIT); diff --git a/xen/arch/arm/arm64/sve.c b/xen/arch/arm/arm64/sve.c index e05ccc38a896..a9144e48ef6b 100644 --- a/xen/arch/arm/arm64/sve.c +++ b/xen/arch/arm/arm64/sve.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -49,6 +50,17 @@ register_t compute_max_zcr(void) return vl_to_zcr(hw_vl); } +/* Get the system sanitized value for VL in bits */ +unsigned int get_sys_vl_len(void) +{ +if ( !cpu_has_sve ) +return 0; + +/* ZCR_ELx len field is ((len + 1) * 128) = vector bits length */ +return ((system_cpuinfo.zcr64.bits[0] & ZCR_ELx_LEN_MASK) + 1U) * +SVE_VL_MULTIPLE_VAL; +} + /* * Local variables: * mode: C diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index d5ab15db46c4..6c22551b0ed2 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -13,6 +13,7 @@ #include #include +#include #include #include #include @@ -555,6 +556,8 @@ int arch_vcpu_create(struct vcpu *v) v->arch.vmpidr = MPIDR_SMP | vcpuid_to_vaffinity(v->vcpu_id); v->arch.cptr_el2 =
RE: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map
Hi Jan, > -Original Message- > Subject: Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device > tree NUMA distance map > > The [2] link you provided discusses NUMA_LOCAL_DISTANCE. > >>> > >>> I inferred the discussion as "we should try to keep consistent between the > >> value > >>> used in device tree and ACPI tables". Maybe my inference is wrong. > >>> > Looking at > Linux'es Documentation/devicetree/numa.txt, there's no mention of an > upper bound on the distance values. It only says that on the diagonal > entries should be 10 (i.e. matching ACPI, without really saying so). > >>> > >>> I agree that the NUMA device tree binding is a little bit vague. So I > >>> cannot > >>> say the case that you provided is not valid. I would like to ask Arm > >> maintainers > >>> (putting them into To:) opinion on this as I think I am not the one to > decide > >> the > >>> expected behavior on Arm. > >>> > >>> Bertrand/Julien/Stefano: Would you please kindly share your opinion on > >> which > >>> value should be used as the default value of the node distance map? Do > >> you > >>> think reusing the "unreachable" distance, i.e. 0xFF, as the default node > >> distance > >>> is acceptable here? Thanks! > >> > >> My suggestion would be to, rather than rejecting values >= 0xff, to > >> saturate > >> at 0xfe, while keeping 0xff for NUMA_NO_DISTANCE (and overall keeping > >> things > >> consistent with ACPI). > > > > Since it has been a while and there were no feedback from Arm > maintainers, > > I would like to follow your suggestions for v5. However while I was writing > the > > code for the "saturation", i.e., adding below snippet in > numa_set_distance(): > > ``` > > if ( distance > NUMA_NO_DISTANCE ) > > distance = NUMA_MAX_DISTANCE; > > ``` > > I noticed an issue which needs your clarification: > > Currently, the default value of the distance map is NUMA_NO_DISTANCE, > > which indicates the nodes are not reachable. IMHO, if in device tree, we do > > saturations like above for ACPI invalid distances (distances >= 0xff), by > saturating > > the distance to 0xfe, we are making the unreachable nodes to reachable. I > am > > not sure if this will lead to problems. Do you have any more thoughts? > Thanks! > > I can only answer this with a question back: How is "unreachable" > represented > in DT? Exactly, that is also what we I am trying to find but failed. My understanding is that, the spec of NUMA is defined in the ACPI, and the DT NUMA binding only specifies how users can use DT to represent the same set of ACPI data, instead of define another standard. By looking into the existing implementation of NUMA for DT, in Linux, from drivers/base/arch_numa.c: numa_set_distance(), there is a "if ((u8)distance != distance)" then return. So I think at least in Linux the distance value is consistent with the ACPI spec. > Or is "unreachable" simply expressed by the absence of any data? Maybe I am wrong but I don't think so, as in the Linux implementation, drivers/of/of_numa.c: of_numa_parse_distance_map_v1(), the for loop "for (i = 0; i + 2 < entry_count; i += 3)" basically implies no fields should be omitted in the distance map entry. Kind regards, Henry > > Jan
[xen-unstable test] 180900: tolerable FAIL - PUSHED
flight 180900 xen-unstable real [real] flight 180911 xen-unstable real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/180900/ http://logs.test-lab.xenproject.org/osstest/logs/180911/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemuu-win7-amd64 12 windows-install fail pass in 180911-retest Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stop fail in 180911 like 180884 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180884 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180884 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180884 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 180884 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 180884 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 180884 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 180884 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 180884 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180884 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180884 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180884 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass version targeted for testing: xen c7908869ac26961a3919491705e521179ad3fc0e baseline version: xen 753d903a6f2d1e68d98487d36449b5739c28d65a Last test of basis 180884 2023-05-22 01:52:01
Re: [PATCH RFC] build: respect top-level .config also for out-of-tree hypervisor builds
On 22.05.2023 17:49, Anthony PERARD wrote: > On Mon, May 08, 2023 at 08:23:43AM +0200, Jan Beulich wrote: >> On 05.05.2023 18:08, Anthony PERARD wrote: >>> On Wed, Mar 15, 2023 at 03:58:59PM +0100, Jan Beulich wrote: With in-tree builds Config.mk includes a .config file (if present) from the top-level directory. Similar functionality is wanted with out-of- tree builds. Yet the concept of "top-level directory" becomes fuzzy in that case, because there is not really a requirement to have identical top-level directory structure in the output tree; in fact there's no need for anything top-level-ish there. Look for such a .config, but only if the tree layout matches (read: if the directory we're building in is named "xen"). >>> >>> Well, as long as the "xen/" part of the repository is the only build >>> system to be able to build out-of-srctree, there isn't going to be a >>> top-level .config possible in the build tree, as such .config will be >>> outside of the build tree. Reading outside of the build tree or source >>> tree might be problematic. >>> >>> It's a possibility that some project might want to just build the >>> hypervisor, and they happened to name the build tree "xen", but they >>> also have a ".config" use for something else. Kconfig is using ".config" >>> for other reason for example, like we do to build Xen. >> >> Right, that's what my first RFC remark is about. >> >>> It might be better to have a different name instead of ".config", and >>> putting it in the build tree rather than the parent directory. Maybe >>> ".xenbuild-config" ? >> >> Using a less ambiguous name is clearly an option. Putting the file in >> the (Xen) build directory, otoh, imo isn't: In the hope that further >> sub-trees would be enabled to build out-of-tree (qemu for instance in >> principle can already, and I guess at least some of stubdom's sub- >> packages also can), the present functionality of the top-level >> .config (or whatever its new name) also controlling those builds would >> better be retained. > > I'm not sure what out-of-tree build for the whole tree will look like, > but it probably going to be `/path/to/configure && make`. After that, > Config.mk should know what kind of build it's doing, and probably only > load ".config" in the build tree. Except that the hypervisor build still isn't really connected to ./configure's results. > In the meantime, it feels out of place > for xen/Makefile or xen/Rules.mk to load a ".config" that would be > present in the parent directory of the build dir. Right, hence me looking for possible alternatives (and using this approach only for the apparent lack thereof). >>> I've been using the optional makefile named "xen-version" to adjust >>> variable of the build system, with content like: >>> >>> export XEN_TARGET_ARCH=arm64 >>> export CROSS_COMPILE=aarch64-linux-gnu- >>> >>> so that I can have multiple build tree for different arch, and not have >>> to do anything other than running make and do the expected build. Maybe >>> that's not possible because for some reason, the build system still >>> reconfigure some variable when entering a sub-directory, but that's a >>> start. >> >> Hmm, interesting idea. I could (ab)use this at least in the short >> term. Yet even then the file would further need including from >> Rules.mk. Requiring all variables defined there to be exported isn't >> a good idea, imo. Iirc not all make variables can usefully be >> exported. For example, I have a local extension to CROSS_COMPILE in >> place, which uses a variable with a dash in its name. >> Signed-off-by: Jan Beulich --- RFC: The directory name based heuristic of course isn't nice. But I couldn't think of anything better. Suggestions? >>> >>> I can only thing of looking at a file that is in the build-tree rather >>> than outside of it. A file named ".xenbuild-config" proposed early for >>> example. >>> RFC: There also being a .config in the top-level source dir would be a little problematic: It would be included _after_ the one in the object tree. Yet if such a scenario is to be expected/supported at all, it makes more sense the other way around. >>> >>> Well, that would mean teaching Config.mk about out-of-tree build that >>> part of the repository is capable of, or better would be to stop trying >>> to read ".config" from Config.mk, and handle the file in the different >>> sub-build systems. >> >> Hmm, is that a viable option? Or put differently: Wouldn't this mean doing >> away with ./Config.mk altogether? Which in turn would mean no central >> place anymore where XEN_TARGET_ARCH and friends could be overridden. (I >> view this as a capability that we want to retain, if nothing else then for >> the - see above - future option of allowing more than just xen/ to be >> built out-of-tree.) > > No, I'm not trying to get rid of Config.mk. There's a few thing in it > that I'd like to
Re: PVH Dom0 related UART failure
On 23.05.2023 00:20, Stefano Stabellini wrote: > On Sat, 20 May 2023, Roger Pau Monné wrote: >> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c >> index ec2e978a4e6b..0ff8e940fa8d 100644 >> --- a/xen/drivers/vpci/header.c >> +++ b/xen/drivers/vpci/header.c >> @@ -289,6 +289,13 @@ static int modify_bars(const struct pci_dev *pdev, >> uint16_t cmd, bool rom_only) >> */ >> for_each_pdev ( pdev->domain, tmp ) >> { >> +if ( !tmp->vpci ) >> +{ >> +printk(XENLOG_G_WARNING "%pp: not handled by vPCI for %pd\n", >> + >sbdf, pdev->domain); >> +continue; >> +} >> + >> if ( tmp == pdev ) >> { >> /* >> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c >> index 652807a4a454..0baef3a8d3a1 100644 >> --- a/xen/drivers/vpci/vpci.c >> +++ b/xen/drivers/vpci/vpci.c >> @@ -72,7 +72,12 @@ int vpci_add_handlers(struct pci_dev *pdev) >> unsigned int i; >> int rc = 0; >> >> -if ( !has_vpci(pdev->domain) ) >> +if ( !has_vpci(pdev->domain) || >> + /* >> + * Ignore RO and hidden devices, those are in use by Xen and vPCI >> + * won't work on them. >> + */ >> + pci_get_pdev(dom_xen, pdev->sbdf) ) >> return 0; >> >> /* We should not get here twice for the same device. */ > > > Now this patch works! Thank you!! :-) > > You can check the full logs here > https://gitlab.com/xen-project/people/sstabellini/xen/-/jobs/4329259080 > > Is the patch ready to be upstreamed aside from the commit message? I don't think so. vPCI ought to work on "r/o" devices. Out of curiosity, have you also tried my (hackish and hence RFC) patch [1]? Jan [1] https://lists.xen.org/archives/html/xen-devel/2021-08/msg01489.html
Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map
On 23.05.2023 08:31, Henry Wang wrote: >> -Original Message- >> Subject: Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device >> tree NUMA distance map >> >>> +/* The default value in node_distance_map is NUMA_NO_DISTANCE >> */ >>> +if ( opposite == NUMA_NO_DISTANCE ) >> >> And the matrix you're reading from can't hold NUMA_NO_DISTANCE entries? >> I ask because you don't check this above; you only check against >> NUMA_LOCAL_DISTANCE. > > My understanding for the purpose of this part of code is to check if the opposite > way distance has already been set, so we need to compare the opposite way > distance with the default value NUMA_NO_DISTANCE here. > > Back to your question, I can see your point of the question. However I >> don't think > NUMA_NO_DISTANCE is a valid value to describe the node distance in the device > tree. This is because I hunted down the previous discussions and found >> [2] about > we should try to keep consistent between the value used in device tree >> and ACPI > tables. From the ACPI spec, 0xFF, i.e. NUMA_NO_DISTANCE means unreachable. > I think this is also the reason why NUMA_NO_DISTANCE can be used as >> the default > value of the distance map, otherwise we won't have any value to use. The [2] link you provided discusses NUMA_LOCAL_DISTANCE. >>> >>> I inferred the discussion as "we should try to keep consistent between the >> value >>> used in device tree and ACPI tables". Maybe my inference is wrong. >>> Looking at Linux'es Documentation/devicetree/numa.txt, there's no mention of an upper bound on the distance values. It only says that on the diagonal entries should be 10 (i.e. matching ACPI, without really saying so). >>> >>> I agree that the NUMA device tree binding is a little bit vague. So I cannot >>> say the case that you provided is not valid. I would like to ask Arm >> maintainers >>> (putting them into To:) opinion on this as I think I am not the one to >>> decide >> the >>> expected behavior on Arm. >>> >>> Bertrand/Julien/Stefano: Would you please kindly share your opinion on >> which >>> value should be used as the default value of the node distance map? Do >> you >>> think reusing the "unreachable" distance, i.e. 0xFF, as the default node >> distance >>> is acceptable here? Thanks! >> >> My suggestion would be to, rather than rejecting values >= 0xff, to saturate >> at 0xfe, while keeping 0xff for NUMA_NO_DISTANCE (and overall keeping >> things >> consistent with ACPI). > > Since it has been a while and there were no feedback from Arm maintainers, > I would like to follow your suggestions for v5. However while I was writing > the > code for the "saturation", i.e., adding below snippet in numa_set_distance(): > ``` > if ( distance > NUMA_NO_DISTANCE ) > distance = NUMA_MAX_DISTANCE; > ``` > I noticed an issue which needs your clarification: > Currently, the default value of the distance map is NUMA_NO_DISTANCE, > which indicates the nodes are not reachable. IMHO, if in device tree, we do > saturations like above for ACPI invalid distances (distances >= 0xff), by > saturating > the distance to 0xfe, we are making the unreachable nodes to reachable. I am > not sure if this will lead to problems. Do you have any more thoughts? Thanks! I can only answer this with a question back: How is "unreachable" represented in DT? Or is "unreachable" simply expressed by the absence of any data? Jan
Re: [PATCH v2 00/10] x86: support AVX512-FP16
On 22.05.2023 18:25, Andrew Cooper wrote: > On 03/04/2023 3:56 pm, Jan Beulich wrote: >> While I (quite obviously) don't have any suitable hardware, Intel's >> SDE allows testing the implementation. And since there's no new >> state (registers) associated with this ISA extension, this should >> suffice for integration. > > I've given this a spin on a Sapphire Rapids system. > > Relevant (AFAICT) bits of the log: > > Testing vfpclasspsz $0x46,64(%edx),%k2...okay > Testing vfpclassphz $0x46,128(%ecx),%k3...okay > ... > Testing avx512_fp16/all disp8 handling...okay > Testing avx512_fp16/128 disp8 handling...okay > ... > Testing AVX512_FP16 f16 scal native execution...okay > Testing AVX512_FP16 f16 scal 64-bit code sequence...okay > Testing AVX512_FP16 f16 scal 32-bit code sequence...okay > Testing AVX512_FP16 f16x32 native execution...okay > Testing AVX512_FP16 f16x32 64-bit code sequence...okay > Testing AVX512_FP16 f16x32 32-bit code sequence...okay > Testing AVX512_FP16+VL f16x8 native execution...okay > Testing AVX512_FP16+VL f16x8 64-bit code sequence...okay > Testing AVX512_FP16+VL f16x8 32-bit code sequence...okay > Testing AVX512_FP16+VL f16x16 native execution...okay > Testing AVX512_FP16+VL f16x16 64-bit code sequence...okay > Testing AVX512_FP16+VL f16x16 32-bit code sequence...okay > > and it exits zero, so everything seems fine. > > > One thing however, this series ups the minimum GCC version required to > build the emulator at all: > > make: Entering directory '/local/xen.git/tools/tests/x86_emulator' > gcc: error: unrecognized command-line option ‘-mavx512fp16’; did you > mean ‘-mavx512bf16’? > Makefile:121: Test harness not built, use newer compiler than "gcc" > (version 10) and an "{evex}" capable assembler > > and I'm not sure we want to do this. When upping the version of GCC but > leaving binutils as-was does lead to a build of the harness without > AVX512-FP16 active, which is the preferred behaviour here. Well, this series on its own does, but I did notice the issue already. Hence "x86emul: rework compiler probing in the test harness" [1]. Jan [1] https://lists.xen.org/archives/html/xen-devel/2023-03/msg00123.html
Re: [XEN PATCH v8 03/22] tools: add Arm FF-A mediator
On Thu, May 18, 2023 at 4:35 PM Anthony PERARD wrote: > > On Thu, Apr 13, 2023 at 09:14:05AM +0200, Jens Wiklander wrote: > > Adds a new "ffa" value to the Enumeration "tee_type" to indicate if a > > guest is trusted to use FF-A. > > Is "ffa" working yet in the hypervisor? (At this point in the patch > series) I'm asking because the doc change is at the end of the patch > series and this patch at the beginning. > > I feel like this patch would be better at the end of the series, just > before the doc change, when the hypervisor is ready for it. Makes sense, I'll move it. > > In any case: > Acked-by: Anthony PERARD Thanks, Jens > > Thanks, > > -- > Anthony PERARD
RE: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map
Hi Jan, > -Original Message- > Subject: Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device > tree NUMA distance map > > > +/* The default value in node_distance_map is > >> NUMA_NO_DISTANCE > */ > > +if ( opposite == NUMA_NO_DISTANCE ) > > And the matrix you're reading from can't hold NUMA_NO_DISTANCE > >> entries? > I ask because you don't check this above; you only check against > NUMA_LOCAL_DISTANCE. > >>> > >>> My understanding for the purpose of this part of code is to check if the > >> opposite > >>> way distance has already been set, so we need to compare the opposite > >> way > >>> distance with the default value NUMA_NO_DISTANCE here. > >>> > >>> Back to your question, I can see your point of the question. However I > don't > >> think > >>> NUMA_NO_DISTANCE is a valid value to describe the node distance in the > >> device > >>> tree. This is because I hunted down the previous discussions and found > [2] > >> about > >>> we should try to keep consistent between the value used in device tree > and > >> ACPI > >>> tables. From the ACPI spec, 0xFF, i.e. NUMA_NO_DISTANCE means > >> unreachable. > >>> I think this is also the reason why NUMA_NO_DISTANCE can be used as > the > >> default > >>> value of the distance map, otherwise we won't have any value to use. > >> > >> The [2] link you provided discusses NUMA_LOCAL_DISTANCE. > > > > I inferred the discussion as "we should try to keep consistent between the > value > > used in device tree and ACPI tables". Maybe my inference is wrong. > > > >> Looking at > >> Linux'es Documentation/devicetree/numa.txt, there's no mention of an > >> upper bound on the distance values. It only says that on the diagonal > >> entries should be 10 (i.e. matching ACPI, without really saying so). > > > > I agree that the NUMA device tree binding is a little bit vague. So I cannot > > say the case that you provided is not valid. I would like to ask Arm > maintainers > > (putting them into To:) opinion on this as I think I am not the one to > > decide > the > > expected behavior on Arm. > > > > Bertrand/Julien/Stefano: Would you please kindly share your opinion on > which > > value should be used as the default value of the node distance map? Do > you > > think reusing the "unreachable" distance, i.e. 0xFF, as the default node > distance > > is acceptable here? Thanks! > > My suggestion would be to, rather than rejecting values >= 0xff, to saturate > at 0xfe, while keeping 0xff for NUMA_NO_DISTANCE (and overall keeping > things > consistent with ACPI). Since it has been a while and there were no feedback from Arm maintainers, I would like to follow your suggestions for v5. However while I was writing the code for the "saturation", i.e., adding below snippet in numa_set_distance(): ``` if ( distance > NUMA_NO_DISTANCE ) distance = NUMA_MAX_DISTANCE; ``` I noticed an issue which needs your clarification: Currently, the default value of the distance map is NUMA_NO_DISTANCE, which indicates the nodes are not reachable. IMHO, if in device tree, we do saturations like above for ACPI invalid distances (distances >= 0xff), by saturating the distance to 0xfe, we are making the unreachable nodes to reachable. I am not sure if this will lead to problems. Do you have any more thoughts? Thanks! Kind regards, Henry > > Jan