[ovmf test] 179527: all pass - PUSHED
flight 179527 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/179527/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 8820767fb3bad09eeedecc3030d75c9e0cd4cab7 baseline version: ovmf a0f9628705e35c981ae95376f9ebedf877d09111 Last test of basis 179517 2023-03-09 10:12:20 Z0 days Testing same since 179527 2023-03-10 02:10:45 Z0 days1 attempts People who touched revisions under test: Ashraf Ali S S, Ashraf Ali jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git a0f9628705..8820767fb3 8820767fb3bad09eeedecc3030d75c9e0cd4cab7 -> xen-tested-master
[xen-unstable test] 179522: tolerable trouble: fail/pass/starved - PUSHED
flight 179522 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/179522/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-livepatch 7 xen-install fail in 179513 pass in 179522 test-amd64-i386-xl7 xen-installfail pass in 179513 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-install fail pass in 179513 test-amd64-i386-examine 6 xen-installfail pass in 179513 test-amd64-amd64-xl-rtds 22 guest-start/debian.repeat fail pass in 179513 test-amd64-i386-xl-vhd 21 guest-start/debian.repeat fail pass in 179513 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 12 windows-install fail in 179513 like 179392 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 179503 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 179503 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 179503 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 179503 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 179503 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 179503 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 179503 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 179503 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 179503 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-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-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-examine 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen 7a59096258fb9e9679538da8851fe00656841980 baseline version: xen 31270f11a96ebb875cd70661e2df9e5c6edd7564 Last test of basis 179503 2023-03-08 01:55:30 Z2 days Failing since179509 2023-03-08 14:07:05 Z1 days3 attempts Testing same since 179513 2023-03-09 03:51:00 Z1 days2 attempts People who
Re: [BUG] x2apic broken with current AMD hardware
On Thu, Mar 09, 2023 at 10:03:23AM +0100, Jan Beulich wrote: > On 09.03.2023 00:08, Elliott Mitchell wrote: > > > > As such I'm less than certain the problem is still in HEAD, though > > Neowutran and Co working with 4.16 and the commit log being quiet > > suggests there is a good chance. > > > > More detail, pretty well most things are broken for Domain 0 without > > "x2apic=false". Trying to boot with a 6.1.12 a USB keyboard was > > completely unresponsive, on screen the initial ramdisk script output was > > indicating problems interacting with storage devices. Those two together > > suggested an interrupt issue and adding "x2apic=false" caused domain 0 to > > successfully boot. > > A 5.10 kernel similarly requires "x2apic=false" to successfully boot. > > > > So could be a commit after 4.16 fixed x2apic for current AMD hardware, > > but may still be broken. > > If Dom0 boot is affected, trying a newer hypervisor shouldn't be a problem. > You won't need any of the toolstack to match just to see whether Dom0 boots. > > In any event you will want to collect a serial log at maximum verbosity. > It would also be of interest to know whether turning off the IOMMU avoids > the issue as well (on the assumption that your system has less than 255 > CPUs). Well, I can now state "x2apic=false" IS required for Xen 4.17. Since the last x2apic commit was about a year ago, I believe this matches HEAD. I missed the logs since the USB-serial adapter decided to bugger when the machine rebooted. Is it just me or is https://wiki.xenproject.org/wiki/Xen_Serial_Console out of date? Last time I used serial debugging I recall the options being different. Due to stress level this time I'm not so favorable to looking things up in Git (this system is crucial in my development workflow, so being in a problematic state is trouble). -- (\___(\___(\__ --=> 8-) EHM <=-- __/)___/)___/) \BS (| ehem+sig...@m5p.com PGP 87145445 |) / \_CS\ | _ -O #include O- _ | / _/ 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
[qemu-mainline test] 179518: tolerable trouble: fail/pass/starved - PUSHED
flight 179518 qemu-mainline real [real] flight 179525 qemu-mainline real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/179518/ http://logs.test-lab.xenproject.org/osstest/logs/179525/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-amd64-libvirt-vhd 19 guest-start/debian.repeat fail pass in 179525-retest Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 179501 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 179501 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 179501 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 179501 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 179501 test-amd64-i386-xl-pvshim14 guest-start fail 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-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-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-credit2 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 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-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-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: qemuu7b0f0aa55fd292fa3489755a3a896e496c51ea86 baseline version: qemuu9832009d9dd2386664c15cc70f6e6bfe062be8bd Last test of basis 179501 2023-03-07 23:38:47 Z2 days Testing same since 179518 2023-03-09 10:37:19 Z0 days1 attempts People who touched revisions under test: Paolo Bonzini Peter Maydell jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf starved build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt starved build-i386-libvirt pass
Re: [PATCH v2] hvc/xen: prevent concurrent accesses to the shared ring
Roger Pau Monné writes: > On Mon, Dec 12, 2022 at 01:36:48PM +0100, Roger Pau Monné wrote: >> On Fri, Dec 02, 2022 at 12:40:05PM +0100, Roger Pau Monné wrote: >> > On Wed, Nov 30, 2022 at 05:08:06PM -0800, Stefano Stabellini wrote: >> > > On Wed, 30 Nov 2022, Roger Pau Monne wrote: >> > > > The hvc machinery registers both a console and a tty device based on >> > > > the hv ops provided by the specific implementation. Those two >> > > > interfaces however have different locks, and there's no single locks >> > > > that's shared between the tty and the console implementations, hence >> > > > the driver needs to protect itself against concurrent accesses. >> > > > Otherwise concurrent calls using the split interfaces are likely to >> > > > corrupt the ring indexes, leaving the console unusable. >> > > > >> > > > Introduce a lock to xencons_info to serialize accesses to the shared >> > > > ring. This is only required when using the shared memory console, >> > > > concurrent accesses to the hypercall based console implementation are >> > > > not an issue. >> > > > >> > > > Note the conditional logic in domU_read_console() is slightly modified >> > > > so the notify_daemon() call can be done outside of the locked region: >> > > > it's an hypercall and there's no need for it to be done with the lock >> > > > held. >> > > >> > > For domU_read_console: I don't mean to block this patch but we need to >> > > be sure about the semantics of hv_ops.get_chars. Either it is expected >> > > to be already locked, then we definitely shouldn't add another lock to >> > > domU_read_console. Or it is not expected to be already locked, then we >> > > should add the lock. >> > > >> > > My impression is that it is expected to be already locked, but I think >> > > we need Greg or Jiri to confirm one way or the other. >> > >> > Let me move both to the 'To:' field then. >> > >> > My main concern is the usage of hv_ops.get_chars hook in >> > hvc_poll_get_char(), as it's not obvious to me that callers of >> > tty->poll_get_char hook as returned by tty_find_polling_driver() will >> > always do so with the tty lock held (in fact the only user right now >> > doesn't seem to hold the tty lock). >> > >> > > Aside from that the rest looks fine. > > I guess I could reluctantly remove the lock in the get_chars hook, > albeit I'm not convinced at all the lock is not needed there. I don't know the xen driver, but other HVC backends have a lock around their private state in their get_chars() implementations. See eg. hvterm_raw_get_chars(). cheers
Re: [PATCH 0/7] sysctl: slowly deprecate register_sysctl_table()
On Thu, Mar 02, 2023 at 12:46:05PM -0800, Luis Chamberlain wrote: > I'm happy to take these via sysctl-next [0] but since > I don' think register_sysctl_table() will be nuked on v6.4 I think > it's fine for each of these to go into each respective tree. I can > pick up last stragglers on sysctl-next. If you want me to take this > via sysctl-next too, just let me know and I'm happy to do that. Either > way works. As I noted I've dropped the following already-picked-up patches from my queue: ipmi: simplify sysctl registration sgi-xp: simplify sysctl registration tty: simplify sysctl registration I've taken the rest now through sysctl-next: scsi: simplify sysctl registration with register_sysctl() hv: simplify sysctl registration md: simplify sysctl registration xen: simplify sysctl registration for balloon If a maintainer would prefer to take one on through their tree fine by me too, just let me know and I'll drop the patch. Luis
Re: [help] Xen 4.14.5 on Devuan 4.0 Chimaera, regression from Xen 4.0.1
On 09/03/2023 7:34 pm, tachyon_...@web.de wrote: > Subject: > [help] Xen 4.14.5 on Devuan 4.0 Chimaera > From: > tachyon_...@web.de > Date: > 09/03/2023, 7:34 pm > > To: > xen-devel@lists.xenproject.org > > > Hello. > > Following the advice of Andrew Cooper (thanks for helping out) over on > OFTC.net in #xen, I'll post this here. > If this is the wrong place, please move it to the right section of your > mailing lists. > > I got some problems running Xen 4.14.5 on Devuan 4.0. > > The AMD-Vi and I/O virtualisation are not being enabled when booting up > the host system with Xen. > > Hardware used: > Mainboard: Gigabyte GA-890FXA-UD5, BIOS version F6 (2010.11.24, the > latest version) > CPU: AMD Phenom II X4 910e > Memory: 16GB > Storage: 2TB Crucial MX500 > > A short snippet of what I see when invoking "xl dmesg": > > (XEN) No southbridge IO-APIC found in IVRS table > (XEN) AMD-Vi: Error initialization > (XEN) I/O virtualisation disabled > > What I would like to see (taken from Xen 4.0.1 running on Debian > Squeeze, in use since 2011): > > (XEN) IOAPIC[0]: apic_id 8, version 33, address 0xfec0, GSI 0-23 > (XEN) Enabling APIC mode: Flat. Using 1 I/O APICs > (XEN) Using scheduler: SMP Credit Scheduler (credit) > (XEN) Detected 2611.936 MHz processor. > (XEN) Initing memory sharing. > (XEN) HVM: ASIDs enabled. > (XEN) HVM: SVM enabled > (XEN) HVM: Hardware Assisted Paging detected. > (XEN) AMD-Vi: IOMMU 0 Enabled. > (XEN) I/O virtualisation enabled > > My question would be if this is "normal" behaviour due to older hardware > being used with newer versions of Xen (compared to the old 4.0.1) or if > this is a bug. > If the latter, has this been addressed already in newer version (4.14+)? > > I'll attach some log files (hypervisor.log, dom0.log, xl_info.log, > lspci_vvv.log, acpi.dmp, ivrs.dat, ivrs.dsl). > > Thank you for your time. Let me braindump the investigation so far before I forget it. Xen requires that there is an IVRS special-device record describing an IO-APIC 00:14.0. This check failing is the source of the "No southbridge" message, and the cause of the IOMMU(s) being turned off. The MADT and IVRS tables agree that there is one IO-APIC in the system, but that's the northbridge IO-APIC, not the southbridge. The block diagram for the southbridge does have a PIC/IO-APIC as part of the PCI bridge, so honestly I was expecting the MADT to describe 2 IO-APICs. But OTOH, I could see this legitimately not existing in configurations where the PCI bridge isn't in use. `xl dmesg` does have a few unknown irqs, so there might be something down in the southbridge really generating interrupts. Or there might be a IRQ misconfiguration elsewhere, and this is just a red herring. However, a consequence of the northbridge and southbridge being separate chips means that all southbridge IO is fully encapsulated by the IOMMU in the northbridge. So irrespective of whether there is ah IO-APIC operating properly in the southbridge, and whether or not it's properly described, I think Xen's insistence that there must be an IVRS special-device entry for it is bogus. Furthermore, Xen's decisions are monumentally stupid. It takes a specifically safe (IOMMU-wise) system, and because it can't figure out a partial aspect of interrupt handling the southbridge, decided that the system can't be safe (a false conclusion) and turns the IOMMU off fully to compensate, which makes the system concretely less safe. ~Andrew
[linux-linus test] 179516: regressions - trouble: fail/pass/starved
flight 179516 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/179516/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-pair25 guest-start/debian fail REGR. vs. 178042 test-amd64-amd64-xl-credit1 20 guest-localmigrate/x10 fail REGR. vs. 178042 test-amd64-amd64-xl 14 guest-start fail REGR. vs. 178042 test-amd64-amd64-xl-xsm 18 guest-localmigrate fail REGR. vs. 178042 test-amd64-amd64-xl-pvhv2-amd 17 guest-saverestore fail REGR. vs. 178042 test-amd64-amd64-freebsd11-amd64 13 guest-start fail REGR. vs. 178042 test-amd64-amd64-libvirt 14 guest-start fail REGR. vs. 178042 test-amd64-amd64-libvirt-pair 28 guest-migrate/dst_host/src_host/debian.repeat fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 12 debian-hvm-install fail REGR. vs. 178042 test-amd64-coresched-amd64-xl 14 guest-start fail REGR. vs. 178042 test-arm64-arm64-xl-credit1 14 guest-start fail REGR. vs. 178042 test-arm64-arm64-libvirt-xsm 17 guest-stop fail REGR. vs. 178042 test-arm64-arm64-xl-thunderx 18 guest-start/debian.repeat fail REGR. vs. 178042 test-arm64-arm64-xl 14 guest-start fail REGR. vs. 178042 test-amd64-amd64-qemuu-nested-intel 12 debian-hvm-install fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 12 debian-hvm-install fail REGR. vs. 178042 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 12 debian-hvm-install fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 12 debian-hvm-install fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-debianhvm-amd64 12 debian-hvm-install fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-ovmf-amd64 12 debian-hvm-install fail REGR. vs. 178042 test-amd64-amd64-xl-qemut-debianhvm-amd64 12 debian-hvm-install fail REGR. vs. 178042 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 12 debian-hvm-install fail REGR. vs. 178042 test-amd64-amd64-xl-multivcpu 18 guest-localmigrate fail REGR. vs. 178042 test-amd64-amd64-dom0pvh-xl-intel 14 guest-start fail REGR. vs. 178042 test-amd64-amd64-xl-vhd 12 debian-di-installfail REGR. vs. 178042 test-amd64-amd64-pygrub 12 debian-di-installfail REGR. vs. 178042 test-amd64-amd64-libvirt-raw 12 debian-di-installfail REGR. vs. 178042 test-amd64-amd64-libvirt-qcow2 12 debian-di-install fail REGR. vs. 178042 test-arm64-arm64-xl-vhd 12 debian-di-installfail REGR. vs. 178042 test-arm64-arm64-libvirt-raw 12 debian-di-installfail REGR. vs. 178042 test-amd64-amd64-freebsd12-amd64 16 guest-saverestore fail in 179511 REGR. vs. 178042 test-amd64-amd64-xl-pvshim 18 guest-localmigrate fail in 179511 REGR. vs. 178042 test-amd64-amd64-xl-shadow 18 guest-localmigrate fail in 179511 REGR. vs. 178042 test-amd64-amd64-xl-pvhv2-intel 19 guest-saverestore.2 fail in 179511 REGR. vs. 178042 test-amd64-amd64-dom0pvh-xl-amd 17 guest-saverestore fail in 179511 REGR. vs. 178042 test-amd64-amd64-libvirt-xsm 18 guest-saverestore.2 fail in 179511 REGR. vs. 178042 test-amd64-amd64-xl-credit2 22 guest-start/debian.repeat fail in 179511 REGR. vs. 178042 test-arm64-arm64-xl-credit2 17 guest-stop fail in 179511 REGR. vs. 178042 test-amd64-amd64-qemuu-nested-amd 13 nested-setup fail in 179511 REGR. vs. 178042 test-arm64-arm64-xl-xsm 18 guest-start/debian.repeat fail in 179511 REGR. vs. 178042 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 18 guest-localmigrate/x10 fail in 179511 REGR. vs. 178042 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-credit1 18 guest-localmigrate fail in 179511 pass in 179516 test-amd64-amd64-xl-xsm 14 guest-start fail in 179511 pass in 179516 test-amd64-amd64-libvirt-pair 25 guest-start/debian fail in 179511 pass in 179516 test-arm64-arm64-xl-thunderx 14 guest-start fail in 179511 pass in 179516 test-amd64-amd64-xl-multivcpu 14 guest-start fail in 179511 pass in 179516 test-amd64-amd64-freebsd12-amd64 13 guest-startfail pass in 179511 test-amd64-amd64-xl-shadow 14 guest-startfail pass in 179511 test-amd64-amd64-xl-pvshim 14 guest-startfail pass in 179511 test-amd64-amd64-xl-pvhv2-intel 14 guest-start fail pass in 179511 test-amd64-amd64-dom0pvh-xl-amd 14 guest-start fail pass in 179511 test-amd64-amd64-libvirt-xsm 14 guest-startfail pass in 179511 test-amd64-amd64-xl-credit2 19 guest-saverestore.2fail pass in 179511 test-arm64-arm64-xl-xsm 14 guest-startfail pass in 179511 test-arm64-arm64-xl-credit2 14 guest-startfail pass in 179511 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 17 guest-saverestore.2 fail pass in 179511 test-amd64-amd64-xl-rtds 14
[PATCH] tests/vpci: install test
Introduce an install target, like it's used by other tests. This allows running the test on the installed systems, which is easier than running it during the build phase when dealing with automated testing. Strictly speaking the vpci test doesn't require to be run on a Xen host currently, but that allows easier integration with logic that runs the rest of the tests. While there also adjust the makefile to use $(RM), and rename the resulting binary to use a dash instead of an underscore (again to match the rest of the tests). Since the resulting test binary is now part of the distribution CC must be used instead of HOSTCC. Signed-off-by: Roger Pau Monné --- XenRT has recently gained the ability to run the tests in tools/tests that are installed, so the install target is needed for that use-case. --- tools/tests/vpci/Makefile | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tools/tests/vpci/Makefile b/tools/tests/vpci/Makefile index 5075bc2be2..3b85b689d3 100644 --- a/tools/tests/vpci/Makefile +++ b/tools/tests/vpci/Makefile @@ -1,7 +1,7 @@ XEN_ROOT=$(CURDIR)/../../.. include $(XEN_ROOT)/tools/Rules.mk -TARGET := test_vpci +TARGET := test-vpci .PHONY: all all: $(TARGET) @@ -11,17 +11,23 @@ run: $(TARGET) ./$(TARGET) $(TARGET): vpci.c vpci.h list.h main.c emul.h - $(HOSTCC) -g -o $@ vpci.c main.c + $(CC) -o $@ vpci.c main.c .PHONY: clean clean: - rm -rf $(TARGET) *.o *~ vpci.h vpci.c list.h + $(RM) -- $(TARGET) *.o *~ vpci.h vpci.c list.h .PHONY: distclean distclean: clean .PHONY: install -install: +install: all + $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN) + $(INSTALL_PROG) $(TARGET) $(DESTDIR)$(LIBEXEC_BIN) + +.PHONY: uninstall +uninstall: + $(RM) -- $(DESTDIR)$(LIBEXEC_BIN)/$(TARGET) vpci.c: $(XEN_ROOT)/xen/drivers/vpci/vpci.c # Remove includes and add the test harness header -- 2.39.0
[PATCH v2 2/2] arch/arm: time: Add support for parsing interrupts by names
From: Andrei Cherechesu
Added support for parsing the ARM generic timer interrupts DT
node by the "interrupt-names" property, if it is available.
If not available, the usual parsing based on the expected
IRQ order is performed.
Also changed to treating returning 0 as an error case for the
platform_get_irq() calls, since it is not a valid PPI ID and
treating it as a valid case would only cause Xen to BUG() later,
during vgic_reserve_virq().
Added the "hyp-virt" PPI to the timer PPI list, even
though it's currently not in use. If the "hyp-virt" PPI is
not found, the hypervisor won't panic.
Signed-off-by: Andrei Cherechesu
---
xen/arch/arm/include/asm/time.h | 3 ++-
xen/arch/arm/time.c | 27 +++
2 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/xen/arch/arm/include/asm/time.h b/xen/arch/arm/include/asm/time.h
index 4b401c1110..49ad8c1a6d 100644
--- a/xen/arch/arm/include/asm/time.h
+++ b/xen/arch/arm/include/asm/time.h
@@ -82,7 +82,8 @@ enum timer_ppi
TIMER_PHYS_NONSECURE_PPI = 1,
TIMER_VIRT_PPI = 2,
TIMER_HYP_PPI = 3,
-MAX_TIMER_PPI = 4,
+TIMER_HYP_VIRT_PPI = 4,
+MAX_TIMER_PPI = 5,
};
/*
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 433d7be909..868e03ecf6 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -38,6 +38,14 @@ uint32_t __read_mostly timer_dt_clock_frequency;
static unsigned int timer_irq[MAX_TIMER_PPI];
+static const char *timer_irq_names[MAX_TIMER_PPI] = {
+[TIMER_PHYS_SECURE_PPI] = "sec-phys",
+[TIMER_PHYS_NONSECURE_PPI] = "phys",
+[TIMER_VIRT_PPI] = "virt",
+[TIMER_HYP_PPI] = "hyp-phys",
+[TIMER_HYP_VIRT_PPI] = "hyp-virt",
+};
+
unsigned int timer_get_irq(enum timer_ppi ppi)
{
ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI);
@@ -149,15 +157,26 @@ static void __init init_dt_xen_time(void)
{
int res;
unsigned int i;
+bool has_names;
+
+has_names = dt_property_read_bool(timer, "interrupt-names");
/* Retrieve all IRQs for the timer */
for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
{
-res = platform_get_irq(timer, i);
-
-if ( res < 0 )
+if ( has_names )
+res = platform_get_irq_byname(timer, timer_irq_names[i]);
+else
+res = platform_get_irq(timer, i);
+
+if ( res > 0 )
+timer_irq[i] = res;
+/*
+ * Do not panic if "hyp-virt" PPI is not found, since it's not
+ * currently used.
+ */
+else if ( i != TIMER_HYP_VIRT_PPI )
panic("Timer: Unable to retrieve IRQ %u from the device tree\n",
i);
-timer_irq[i] = res;
}
}
--
2.35.1
[PATCH v2 1/2] arch/arm: irq: Add platform_get_irq_byname() implementation
From: Andrei Cherechesu
Moved implementation for the function which parses the IRQs of a DT
node by the "interrupt-names" property from the SMMU-v3 driver
to the IRQ core code and made it non-static to be used as helper.
Also changed it to receive a "struct dt_device_node*" as parameter,
like its counterpart, platform_get_irq(). Updated its usage inside
the SMMU-v3 driver accordingly.
Signed-off-by: Andrei Cherechesu
Reviewed-by: Bertrand Marquis
---
xen/arch/arm/include/asm/irq.h| 2 ++
xen/arch/arm/irq.c| 14 +++
xen/drivers/passthrough/arm/smmu-v3.c | 35 +--
3 files changed, 22 insertions(+), 29 deletions(-)
diff --git a/xen/arch/arm/include/asm/irq.h b/xen/arch/arm/include/asm/irq.h
index 245f49dcba..af94f41994 100644
--- a/xen/arch/arm/include/asm/irq.h
+++ b/xen/arch/arm/include/asm/irq.h
@@ -89,6 +89,8 @@ int irq_set_type(unsigned int irq, unsigned int type);
int platform_get_irq(const struct dt_device_node *device, int index);
+int platform_get_irq_byname(struct dt_device_node *np, const char *name);
+
void irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_mask);
/*
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 79718f68e7..ded495792b 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -718,6 +718,20 @@ int platform_get_irq(const struct dt_device_node *device,
int index)
return irq;
}
+int platform_get_irq_byname(struct dt_device_node *np, const char *name)
+{
+ int index;
+
+ if ( unlikely(!name) )
+ return -EINVAL;
+
+ index = dt_property_match_string(np, "interrupt-names", name);
+ if ( index < 0 )
+ return index;
+
+ return platform_get_irq(np, index);
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/drivers/passthrough/arm/smmu-v3.c
b/xen/drivers/passthrough/arm/smmu-v3.c
index d58c5cd0bf..bfdb62b395 100644
--- a/xen/drivers/passthrough/arm/smmu-v3.c
+++ b/xen/drivers/passthrough/arm/smmu-v3.c
@@ -200,30 +200,6 @@ static inline void dev_iommu_priv_set(struct device *dev,
void *priv)
fwspec->iommu_priv = priv;
}
-static int platform_get_irq_byname_optional(struct device *dev,
- const char *name)
-{
- int index, ret;
- struct dt_device_node *np = dev_to_dt(dev);
-
- if (unlikely(!name))
- return -EINVAL;
-
- index = dt_property_match_string(np, "interrupt-names", name);
- if (index < 0) {
- dev_info(dev, "IRQ %s not found\n", name);
- return index;
- }
-
- ret = platform_get_irq(np, index);
- if (ret < 0) {
- dev_err(dev, "failed to get irq index %d\n", index);
- return -ENODEV;
- }
-
- return ret;
-}
-
/* Start of Linux SMMUv3 code */
static bool disable_bypass = 1;
@@ -2434,6 +2410,7 @@ static int arm_smmu_device_probe(struct platform_device
*pdev)
int irq, ret;
paddr_t ioaddr, iosize;
struct arm_smmu_device *smmu;
+ struct dt_device_node *np = dev_to_dt(pdev);
smmu = xzalloc(struct arm_smmu_device);
if (!smmu)
@@ -2451,7 +2428,7 @@ static int arm_smmu_device_probe(struct platform_device
*pdev)
}
/* Base address */
- ret = dt_device_get_address(dev_to_dt(pdev), 0, , );
+ ret = dt_device_get_address(np, 0, , );
if (ret)
goto out_free_smmu;
@@ -2484,19 +2461,19 @@ static int arm_smmu_device_probe(struct platform_device
*pdev)
/* Interrupt lines */
- irq = platform_get_irq_byname_optional(pdev, "combined");
+ irq = platform_get_irq_byname(np, "combined");
if (irq > 0)
smmu->combined_irq = irq;
else {
- irq = platform_get_irq_byname_optional(pdev, "eventq");
+ irq = platform_get_irq_byname(np, "eventq");
if (irq > 0)
smmu->evtq.q.irq = irq;
- irq = platform_get_irq_byname_optional(pdev, "priq");
+ irq = platform_get_irq_byname(np, "priq");
if (irq > 0)
smmu->priq.q.irq = irq;
- irq = platform_get_irq_byname_optional(pdev, "gerror");
+ irq = platform_get_irq_byname(np, "gerror");
if (irq > 0)
smmu->gerr_irq = irq;
}
--
2.35.1
[PATCH v2 0/2] Fix ARM Generic Timer interrupt parsing
From: Andrei Cherechesu This 2-patch series fixes the parsing of the ARM Generic Timer interrupts from the device tree. If the generic timer interrupts order in the DT was different than the expected order in Xen code, these interrupts would no longer be correctly parsed and registered by Xen, and would result in boot failure. This method with using "interrupt-names" for the generic timer interrupts instead of having them hardcoded in the DTB in a specific order is the newer approach already implemented in Linux. Xen did not have the necessary code for this approach, and it has been implemented by the means of this patch series. Functionality should remain the same if "interrupt-names" is not present in the Generic Timer DTB node of the platform, but the interrupts should then still be present in the expected "sec-phys", "phys", "virt", "hyp-phys", "hyp-virt" order. If "interrupt-names" is present, now it is also correctly handled. Changes v1->v2: - Rebased on latest staging as of 2023-03-09 - Fixed coding style of comment added in 2nd commit - Added to 2nd commit message explanation as to why 0 should be treated as an error case for platform_get_irq Andrei Cherechesu (2): arch/arm: irq: Add platform_get_irq_byname() implementation arch/arm: time: Add support for parsing interrupts by names xen/arch/arm/include/asm/irq.h| 2 ++ xen/arch/arm/include/asm/time.h | 3 ++- xen/arch/arm/irq.c| 14 +++ xen/arch/arm/time.c | 27 ++--- xen/drivers/passthrough/arm/smmu-v3.c | 35 +-- 5 files changed, 47 insertions(+), 34 deletions(-) -- 2.35.1
Re: [PATCH v2] Input: xen-kbdfront - drop keys to shrink modalias
Hi Jason, On mer., mars 08, 2023 at 11:26, Jason Andryuk wrote: > On Thu, Dec 15, 2022 at 8:54 AM Mattijs Korpershoek > wrote: >> >> On Fri, Dec 09, 2022 at 09:26, Jason Andryuk wrote: >> >> > xen kbdfront registers itself as being able to deliver *any* key since >> > it doesn't know what keys the backend may produce. >> > >> > Unfortunately, the generated modalias gets too large and uevent creation >> > fails with -ENOMEM. >> > >> > This can lead to gdm not using the keyboard since there is no seat >> > associated [1] and the debian installer crashing [2]. >> > >> > Trim the ranges of key capabilities by removing some BTN_* ranges. >> > While doing this, some neighboring undefined ranges are removed to trim >> > it further. >> > >> > An upper limit of KEY_KBD_LCD_MENU5 is still too large. Use an upper >> > limit of KEY_BRIGHTNESS_MENU. >> > >> > This removes: >> > BTN_DPAD_UP(0x220)..BTN_DPAD_RIGHT(0x223) >> > Empty space 0x224..0x229 >> > >> > Empty space 0x28a..0x28f >> > KEY_MACRO1(0x290)..KEY_MACRO30(0x2ad) >> > KEY_MACRO_RECORD_START 0x2b0 >> > KEY_MACRO_RECORD_STOP 0x2b1 >> > KEY_MACRO_PRESET_CYCLE 0x2b2 >> > KEY_MACRO_PRESET1(0x2b3)..KEY_MACRO_PRESET3(0xb5) >> > Empty space 0x2b6..0x2b7 >> > KEY_KBD_LCD_MENU1(0x2b8)..KEY_KBD_LCD_MENU5(0x2bc) >> > Empty space 0x2bd..0x2bf >> > BTN_TRIGGER_HAPPY(0x2c0)..BTN_TRIGGER_HAPPY40(0x2e7) >> > Empty space 0x2e8..0x2ff >> > >> > The modalias shrinks from 2082 to 1550 bytes. >> > >> > A chunk of keys need to be removed to allow the keyboard to be used. >> > This may break some functionality, but the hope is these macro keys are >> > uncommon and don't affect any users. >> > >> > [1] https://github.com/systemd/systemd/issues/22944 >> > [2] https://lore.kernel.org/xen-devel/87o8dw52jc@vps.thesusis.net/T/ >> > >> > Cc: Phillip Susi >> > Cc: sta...@vger.kernel.org >> > Signed-off-by: Jason Andryuk >> >> Reviewed-by: Mattijs Korpershoek > > Thank you, Mattjis. > > Any other thoughts? Can this patch be applied? That's not up to to decide, Dmitry might pick this up or give you a review whenever he has time. > > Thanks, > Jason
[xen-unstable test] 179513: regressions - trouble: fail/pass/starved
flight 179513 xen-unstable real [real] flight 179519 xen-unstable real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/179513/ http://logs.test-lab.xenproject.org/osstest/logs/179519/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-livepatch 7 xen-install fail REGR. vs. 179503 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 12 windows-install fail like 179392 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 179503 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 179503 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 179503 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 179503 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 179503 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 179503 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 179503 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 179503 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-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-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-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-amd64-amd64-libvirt 15 migrate-support-checkfail never pass build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-examine 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen 7a59096258fb9e9679538da8851fe00656841980 baseline version: xen 31270f11a96ebb875cd70661e2df9e5c6edd7564 Last test of basis 179503 2023-03-08 01:55:30 Z1 days Failing since179509 2023-03-08 14:07:05 Z1 days2 attempts Testing same since 179513 2023-03-09 03:51:00 Z0 days1 attempts People who touched revisions under test: Bertrand Marquis Juergen Gross Michal Orzel Roger Pau Monné Stefano Stabellini Stefano Stabellini jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass
Re: [PULL 00/27] Enable PV backends with Xen/KVM emulation
On Tue, 7 Mar 2023 at 18:27, David Woodhouse wrote: > > The following changes since commit 9832009d9dd2386664c15cc70f6e6bfe062be8bd: > > Merge tag 'pull-riscv-to-apply-20230306' of > https://gitlab.com/palmer-dabbelt/qemu into staging (2023-03-07 12:53:00 > +) > > are available in the Git repository at: > > git://git.infradead.org/users/dwmw2/qemu.git refs/tags/xenfv-2 > > for you to fetch changes up to 154eac37190c4d80d29b09c226abd899e397530f: > > docs: Update Xen-on-KVM documentation for PV disk support (2023-03-07 > 17:04:30 +) > > > Tested-by: Paul Durrant > ... on real Xen (master branch, 4.18) with a Debian guest. Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/8.0 for any user-visible changes. -- PMM
Re: [PATCH v1 2/2] arch/arm: time: Add support for parsing interrupts by names
Hi,
> On 9 Mar 2023, at 15:36, Andrei Cherechesu
> wrote:
>
>
>
> On 09/03/2023 15:46, Michal Orzel wrote:
>>
>>
>> On 09/03/2023 13:45, Bertrand Marquis wrote:
>>>
>>>
>>> Hi Michal,
>>>
On 9 Mar 2023, at 13:42, Michal Orzel wrote:
Hi Bertrand,
On 09/03/2023 13:19, Bertrand Marquis wrote:
>
>
> Hi Michal,
>
>> On 9 Mar 2023, at 12:35, Michal Orzel wrote:
>>
>>
>>
>> On 09/03/2023 11:39, Bertrand Marquis wrote:
>>>
>>>
>>> Hi Michal,
>>>
On 9 Mar 2023, at 11:05, Michal Orzel wrote:
On 09/03/2023 09:02, Bertrand Marquis wrote:
>
>
> Hi Stefano,
>
>> On 7 Mar 2023, at 22:02, Stefano Stabellini
>> wrote:
>>
>> On Tue, 7 Mar 2023, Bertrand Marquis wrote:
On 7 Mar 2023, at 11:09, Andrei Cherechesu (OSS)
wrote:
From: Andrei Cherechesu
Added support for parsing the ARM generic timer interrupts DT
node by the "interrupt-names" property, if it is available.
If not available, the usual parsing based on the expected
IRQ order is performed.
Also added the "hyp-virt" PPI to the timer PPI list, even
though it's currently not in use. If the "hyp-virt" PPI is
not found, the hypervisor won't panic.
Signed-off-by: Andrei Cherechesu
---
xen/arch/arm/include/asm/time.h | 3 ++-
xen/arch/arm/time.c | 26 ++
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/xen/arch/arm/include/asm/time.h
b/xen/arch/arm/include/asm/time.h
index 4b401c1110..49ad8c1a6d 100644
--- a/xen/arch/arm/include/asm/time.h
+++ b/xen/arch/arm/include/asm/time.h
@@ -82,7 +82,8 @@ enum timer_ppi
TIMER_PHYS_NONSECURE_PPI = 1,
TIMER_VIRT_PPI = 2,
TIMER_HYP_PPI = 3,
-MAX_TIMER_PPI = 4,
+TIMER_HYP_VIRT_PPI = 4,
+MAX_TIMER_PPI = 5,
};
/*
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 433d7be909..794da646d6 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -38,6 +38,14 @@ uint32_t __read_mostly timer_dt_clock_frequency;
static unsigned int timer_irq[MAX_TIMER_PPI];
+static const char *timer_irq_names[MAX_TIMER_PPI] = {
+[TIMER_PHYS_SECURE_PPI] = "sec-phys",
+[TIMER_PHYS_NONSECURE_PPI] = "phys",
+[TIMER_VIRT_PPI] = "virt",
+[TIMER_HYP_PPI] = "hyp-phys",
+[TIMER_HYP_VIRT_PPI] = "hyp-virt",
+};
+
>>>
>>> I would need some reference or a pointer to some doc to check those.
>>>
unsigned int timer_get_irq(enum timer_ppi ppi)
{
ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI);
@@ -149,15 +157,25 @@ static void __init init_dt_xen_time(void)
{
int res;
unsigned int i;
+bool has_names;
+
+has_names = dt_property_read_bool(timer, "interrupt-names");
/* Retrieve all IRQs for the timer */
for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
{
-res = platform_get_irq(timer, i);
-
-if ( res < 0 )
+if ( has_names )
+res = platform_get_irq_byname(timer,
timer_irq_names[i]);
+else
+res = platform_get_irq(timer, i);
+
+if ( res > 0 )
>>>
>>> The behaviour of the code is changed here compared to the current
>>> version as res = 0 will now generate a panic.
>>>
>>> Some device tree might not specify an interrupt number and just put
>>> 0 and Xen will now panic on those systems.
>>> As I have no idea if such systems exists and the behaviour is
>>> modified
>>> you should justify this and mention it in the commit message or keep
>>> the old behaviour and let 0 go through without a panic.
>>>
>>> @stefano, julien any idea here ? should just keep the old behaviour
>>> ?
>>
>> platform_get_irq returns 0 if the irq is 0. The irq cannot be 0
>> because
>> 0 is reserved for SGIs, not PPIs. So I think it is OK to consider 0
>>
Re: [PATCH v1 2/3] xen/riscv: setup initial pagetables
On Thu, 2023-03-09 at 10:46 +0100, Jan Beulich wrote:
> On 08.03.2023 17:16, Oleksii wrote:
> > On Wed, 2023-03-08 at 16:17 +0100, Jan Beulich wrote:
> > > On 08.03.2023 15:54, Oleksii wrote:
> > > > Actually after my latest experiments it looks that we don't
> > > > need to
> > > > calculate that things at all because for RISC-V it is used
> > > > everywhere
> > > > PC-relative access.
> > > > Thereby it doesn't matter what is an address where Xen was
> > > > loaded
> > > > and
> > > > linker address.
> > > > Right now I found only to cases which aren't PC-relative.
> > > > Please look at the patch below:
> > > > diff --git a/xen/arch/riscv/include/asm/config.h
> > > > b/xen/arch/riscv/include/asm/config.h
> > > > index 763a922a04..e1ba613d81 100644
> > > > --- a/xen/arch/riscv/include/asm/config.h
> > > > +++ b/xen/arch/riscv/include/asm/config.h
> > > > @@ -39,7 +39,7 @@
> > > > name:
> > > > #endif
> > > >
> > > > -#define XEN_VIRT_START _AT(UL, 0x8020)
> > > > +#define XEN_VIRT_START _AT(UL, 0x0020)
> > >
> > > I think this wants to remain the address where Xen actually runs,
> > > and
> > > where Xen is linked to. This ...
> > >
> > > > --- a/xen/arch/riscv/traps.c
> > > > +++ b/xen/arch/riscv/traps.c
> > > > @@ -123,8 +123,14 @@ int do_bug_frame(const struct
> > > > cpu_user_regs
> > > > *regs,
> > > > vaddr_t pc)
> > > > const char *filename, *predicate;
> > > > int lineno;
> > > >
> > > > - static const struct bug_frame* bug_frames[] = {
> > > > - &__start_bug_frames[0],
> > > > + /*
> > > > + * force fill bug_frames array using auipc/addi
> > > > instructions
> > > > to
> > > > + * make addresses in bug_frames PC-relative.
> > > > + */
> > > > + const struct bug_frame * force = (const struct bug_frame
> > > > *)
> > > > &__start_bug_frames[0];
> > > > +
> > > > + const struct bug_frame* bug_frames[] = {
> > > > + force,
> > > > &__stop_bug_frames_0[0],
> > > > &__stop_bug_frames_1[0],
> > > > &__stop_bug_frames_2[0],
> > >
> > > ... array would better be static anyway, and ...
> > >
> > > > The changes related to are only to make
> > > > linker_addr
> > > > !=
> > > > load_address. So:
> > > > 1. The first issue with cpu0_boot_stack in the head.S file.
> > > > When we
> > > > do:
> > > > la sp, cpu0_boot_stack
> > > > Pseudo-instruction la will be transformed to auipc/addi OR
> > > > auipc/l{w|d}.
> > > > It depends on an option: nopic, pic. [1]
> > > >
> > > > So the solution can be the following:
> > > > * As it is done in the patch: add to the start of head.S
> > > > ".option
> > > > nopic"
> > > > * Change la to lla thereby it will be always generated
> > > > "auipc/addi"
> > > > to get an address of variable.
> > > >
> > > > 2. The second issue is with the code in do_bug_frame() with
> > > > bug_frames
> > > > array:
> > > > const struct bug_frame* bug_frames[] = {
> > > > &__start_bug_frames[0],
> > > > &__stop_bug_frames_0[0],
> > > > &__stop_bug_frames_1[0],
> > > > &__stop_bug_frames_2[0],
> > > > &__stop_bug_frames_3[0],
> > > > };
> > > > In this case &{start,stop}bug_frames{,{0-3}} will be changed
> > > > to
> > > > linker address. In case of when load_addr is 0x8020 and
> > > > linker_addr
> > > > is 0x0020 then &{start,stop}bug_frames{,{0-3}} will be
> > > > equal to
> > > > 0x0020 + X.
> > >
> > > ... this "solution" to a problem you introduce by wrongly
> > > modifying
> > > the linked address would then need applying to any other similar
> > > code
> > > pattern found in Xen. Which is (I hope obviously) not a viable
> > > route.
> > > Instead code running before address translation is enable needs
> > > to be
> > > extra careful in what code and data items it accesses, and how.
> > >
> > I modified the linked address only for the experiment ( when
> > load_addr
> > != linker_addr to emulate situation Julien told me about), so it's
> > not
> > something I planned to send as a part of the final patch, and I
> > probably forgot to mention that in my previous mail.
> >
> > It is only one place where we have to do a kind of 'force' and is
> > needed to make the current state of RISC-V Xen work in case we
> > don't
> > have MMU enabled yet and linker_addr != load_addr. All other cases
> > where it is used something from the range (linker_start,
> > linker_end)
> > will be managed by MMU.
> >
> > If we can't use mentioned above solution, we still need to handle
> > the
> > situation when linker_addr != load_addr and MMU isn't enabled yet.
> > Other options to do that:
> > 1. add phys_offset ( | load_addr - linker_addr | ) everywhere where
> > bug_frames array is used: bug_frames[id] + phys_offset
>
> Well, that again special cases a certain data structure. As said
> before,
> you need to be very careful with any C code involved before
> translation
> is enabled. Unless you want to
Re: [PATCH v1 2/2] arch/arm: time: Add support for parsing interrupts by names
On 09/03/2023 15:46, Michal Orzel wrote:
>
>
> On 09/03/2023 13:45, Bertrand Marquis wrote:
>>
>>
>> Hi Michal,
>>
>>> On 9 Mar 2023, at 13:42, Michal Orzel wrote:
>>>
>>> Hi Bertrand,
>>>
>>> On 09/03/2023 13:19, Bertrand Marquis wrote:
Hi Michal,
> On 9 Mar 2023, at 12:35, Michal Orzel wrote:
>
>
>
> On 09/03/2023 11:39, Bertrand Marquis wrote:
>>
>>
>> Hi Michal,
>>
>>> On 9 Mar 2023, at 11:05, Michal Orzel wrote:
>>>
>>>
>>>
>>> On 09/03/2023 09:02, Bertrand Marquis wrote:
Hi Stefano,
> On 7 Mar 2023, at 22:02, Stefano Stabellini
> wrote:
>
> On Tue, 7 Mar 2023, Bertrand Marquis wrote:
>>> On 7 Mar 2023, at 11:09, Andrei Cherechesu (OSS)
>>> wrote:
>>>
>>> From: Andrei Cherechesu
>>>
>>> Added support for parsing the ARM generic timer interrupts DT
>>> node by the "interrupt-names" property, if it is available.
>>>
>>> If not available, the usual parsing based on the expected
>>> IRQ order is performed.
>>>
>>> Also added the "hyp-virt" PPI to the timer PPI list, even
>>> though it's currently not in use. If the "hyp-virt" PPI is
>>> not found, the hypervisor won't panic.
>>>
>>> Signed-off-by: Andrei Cherechesu
>>> ---
>>> xen/arch/arm/include/asm/time.h | 3 ++-
>>> xen/arch/arm/time.c | 26 ++
>>> 2 files changed, 24 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/include/asm/time.h
>>> b/xen/arch/arm/include/asm/time.h
>>> index 4b401c1110..49ad8c1a6d 100644
>>> --- a/xen/arch/arm/include/asm/time.h
>>> +++ b/xen/arch/arm/include/asm/time.h
>>> @@ -82,7 +82,8 @@ enum timer_ppi
>>> TIMER_PHYS_NONSECURE_PPI = 1,
>>> TIMER_VIRT_PPI = 2,
>>> TIMER_HYP_PPI = 3,
>>> -MAX_TIMER_PPI = 4,
>>> +TIMER_HYP_VIRT_PPI = 4,
>>> +MAX_TIMER_PPI = 5,
>>> };
>>>
>>> /*
>>> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
>>> index 433d7be909..794da646d6 100644
>>> --- a/xen/arch/arm/time.c
>>> +++ b/xen/arch/arm/time.c
>>> @@ -38,6 +38,14 @@ uint32_t __read_mostly timer_dt_clock_frequency;
>>>
>>> static unsigned int timer_irq[MAX_TIMER_PPI];
>>>
>>> +static const char *timer_irq_names[MAX_TIMER_PPI] = {
>>> +[TIMER_PHYS_SECURE_PPI] = "sec-phys",
>>> +[TIMER_PHYS_NONSECURE_PPI] = "phys",
>>> +[TIMER_VIRT_PPI] = "virt",
>>> +[TIMER_HYP_PPI] = "hyp-phys",
>>> +[TIMER_HYP_VIRT_PPI] = "hyp-virt",
>>> +};
>>> +
>>
>> I would need some reference or a pointer to some doc to check those.
>>
>>> unsigned int timer_get_irq(enum timer_ppi ppi)
>>> {
>>> ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI);
>>> @@ -149,15 +157,25 @@ static void __init init_dt_xen_time(void)
>>> {
>>> int res;
>>> unsigned int i;
>>> +bool has_names;
>>> +
>>> +has_names = dt_property_read_bool(timer, "interrupt-names");
>>>
>>> /* Retrieve all IRQs for the timer */
>>> for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
>>> {
>>> -res = platform_get_irq(timer, i);
>>> -
>>> -if ( res < 0 )
>>> +if ( has_names )
>>> +res = platform_get_irq_byname(timer,
>>> timer_irq_names[i]);
>>> +else
>>> +res = platform_get_irq(timer, i);
>>> +
>>> +if ( res > 0 )
>>
>> The behaviour of the code is changed here compared to the current
>> version as res = 0 will now generate a panic.
>>
>> Some device tree might not specify an interrupt number and just put
>> 0 and Xen will now panic on those systems.
>> As I have no idea if such systems exists and the behaviour is
>> modified
>> you should justify this and mention it in the commit message or keep
>> the old behaviour and let 0 go through without a panic.
>>
>> @stefano, julien any idea here ? should just keep the old behaviour ?
>
> platform_get_irq returns 0 if the irq is 0. The irq cannot be 0
> because
> 0 is reserved for SGIs, not PPIs. So I think it is OK to consider 0 an
> error.
Problem here is that a DTB might not specify all interrupts and just
put
0 for the one not used (or not available for example if you have no
secure
world).
>>> Xen requires presence of
[libvirt test] 179515: tolerable trouble: pass/starved - PUSHED
flight 179515 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/179515/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 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-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail 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-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 build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: libvirt 3138e204b2e3f4d765af66faf1fc9b566de370dd baseline version: libvirt 6386dd897df596a01e3cf0db9f86d496891564dc Last test of basis 179505 2023-03-08 04:20:25 Z1 days Testing same since 179515 2023-03-09 04:18:47 Z0 days1 attempts People who touched revisions under test: Jonathon Jongsma Ján Tomko Michal Privoznik jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf starved build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt starved 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 starved 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 starved test-arm64-arm64-libvirt-raw pass test-armhf-armhf-libvirt-raw starved 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 osstest in general, is at
Re: [PATCH v7 2/5] xen/arm: remove unused defines in
On 09.03.2023 14:33, Oleksii Kurochko wrote: > The following defines BUG_DISP_WIDTH, BUG_LINE_LO_WIDTH, > BUG_LINE_HI_WIDTH aren't used in ARM so could be purged > as unused. Requested-by: Jan Beulich or (but it's not really a bug) Reported-by: Jan Beulich > Signed-off-by: Oleksii Kurochko Reviewed-by: Jan Beulich
Re: [PATCH v1 2/2] arch/arm: time: Add support for parsing interrupts by names
On 09/03/2023 13:45, Bertrand Marquis wrote:
>
>
> Hi Michal,
>
>> On 9 Mar 2023, at 13:42, Michal Orzel wrote:
>>
>> Hi Bertrand,
>>
>> On 09/03/2023 13:19, Bertrand Marquis wrote:
>>>
>>>
>>> Hi Michal,
>>>
On 9 Mar 2023, at 12:35, Michal Orzel wrote:
On 09/03/2023 11:39, Bertrand Marquis wrote:
>
>
> Hi Michal,
>
>> On 9 Mar 2023, at 11:05, Michal Orzel wrote:
>>
>>
>>
>> On 09/03/2023 09:02, Bertrand Marquis wrote:
>>>
>>>
>>> Hi Stefano,
>>>
On 7 Mar 2023, at 22:02, Stefano Stabellini
wrote:
On Tue, 7 Mar 2023, Bertrand Marquis wrote:
>> On 7 Mar 2023, at 11:09, Andrei Cherechesu (OSS)
>> wrote:
>>
>> From: Andrei Cherechesu
>>
>> Added support for parsing the ARM generic timer interrupts DT
>> node by the "interrupt-names" property, if it is available.
>>
>> If not available, the usual parsing based on the expected
>> IRQ order is performed.
>>
>> Also added the "hyp-virt" PPI to the timer PPI list, even
>> though it's currently not in use. If the "hyp-virt" PPI is
>> not found, the hypervisor won't panic.
>>
>> Signed-off-by: Andrei Cherechesu
>> ---
>> xen/arch/arm/include/asm/time.h | 3 ++-
>> xen/arch/arm/time.c | 26 ++
>> 2 files changed, 24 insertions(+), 5 deletions(-)
>>
>> diff --git a/xen/arch/arm/include/asm/time.h
>> b/xen/arch/arm/include/asm/time.h
>> index 4b401c1110..49ad8c1a6d 100644
>> --- a/xen/arch/arm/include/asm/time.h
>> +++ b/xen/arch/arm/include/asm/time.h
>> @@ -82,7 +82,8 @@ enum timer_ppi
>> TIMER_PHYS_NONSECURE_PPI = 1,
>> TIMER_VIRT_PPI = 2,
>> TIMER_HYP_PPI = 3,
>> -MAX_TIMER_PPI = 4,
>> +TIMER_HYP_VIRT_PPI = 4,
>> +MAX_TIMER_PPI = 5,
>> };
>>
>> /*
>> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
>> index 433d7be909..794da646d6 100644
>> --- a/xen/arch/arm/time.c
>> +++ b/xen/arch/arm/time.c
>> @@ -38,6 +38,14 @@ uint32_t __read_mostly timer_dt_clock_frequency;
>>
>> static unsigned int timer_irq[MAX_TIMER_PPI];
>>
>> +static const char *timer_irq_names[MAX_TIMER_PPI] = {
>> +[TIMER_PHYS_SECURE_PPI] = "sec-phys",
>> +[TIMER_PHYS_NONSECURE_PPI] = "phys",
>> +[TIMER_VIRT_PPI] = "virt",
>> +[TIMER_HYP_PPI] = "hyp-phys",
>> +[TIMER_HYP_VIRT_PPI] = "hyp-virt",
>> +};
>> +
>
> I would need some reference or a pointer to some doc to check those.
>
>> unsigned int timer_get_irq(enum timer_ppi ppi)
>> {
>> ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI);
>> @@ -149,15 +157,25 @@ static void __init init_dt_xen_time(void)
>> {
>> int res;
>> unsigned int i;
>> +bool has_names;
>> +
>> +has_names = dt_property_read_bool(timer, "interrupt-names");
>>
>> /* Retrieve all IRQs for the timer */
>> for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
>> {
>> -res = platform_get_irq(timer, i);
>> -
>> -if ( res < 0 )
>> +if ( has_names )
>> +res = platform_get_irq_byname(timer,
>> timer_irq_names[i]);
>> +else
>> +res = platform_get_irq(timer, i);
>> +
>> +if ( res > 0 )
>
> The behaviour of the code is changed here compared to the current
> version as res = 0 will now generate a panic.
>
> Some device tree might not specify an interrupt number and just put
> 0 and Xen will now panic on those systems.
> As I have no idea if such systems exists and the behaviour is modified
> you should justify this and mention it in the commit message or keep
> the old behaviour and let 0 go through without a panic.
>
> @stefano, julien any idea here ? should just keep the old behaviour ?
platform_get_irq returns 0 if the irq is 0. The irq cannot be 0 because
0 is reserved for SGIs, not PPIs. So I think it is OK to consider 0 an
error.
>>>
>>> Problem here is that a DTB might not specify all interrupts and just put
>>> 0 for the one not used (or not available for example if you have no
>>> secure
>>> world).
>> Xen requires presence of EL3,EL2 and on such system, at least the
>> following timers needs to be there
>> according to Arm ARM:
>> - EL3 phys (if EL3 is there)
>
> This might be needed by EL3 but not by Xen.
Xen
[PATCH v7 1/5] xen: introduce CONFIG_GENERIC_BUG_FRAME
A large part of the content of the bug.h is repeated among all architectures, so it was decided to create a new config CONFIG_GENERIC_BUG_FRAME. The version of from x86 was taken as the base version. The patch introduces the following stuff: * common bug.h header * generic implementation of do_bug_frame * new config CONFIG_GENERIC_BUG_FRAME Signed-off-by: Oleksii Kurochko --- Changes in V7: * fix code style. * Remove '#include ' from bug.c. it should be a part of . * move BUILD_BUG_ON_LINE_WIDTH to '#ifndef BUG_FRAME_STRUCT' and define dummy BUILD_BUG_ON_LINE_WIDTH when BUG_FRAME_STRUCT is defined. * remove update prototype of 'void (*fn)(const struct cpu_user_regs *)' to 'void (*fn)(struct cpu_user_regs *)'. * add code to to make sure the type used in run_in_exception_handler() matches the one used here. --- Changes in V6: * fix code style. * change -EINVAL to -ENOENT in case when bug_frame wasn't found in generic do_bug_frame() * change all 'return id' to 'break' inside switch/case of generic do_bug_frame() * move up "#ifndef __ASSEMBLY__" to include BUG_DEBUGGER_TRAP_FATAL * update the comment of BUG_ASM_CONST * make the line with 'BUILD_BUG_ON((line) >> (BUG_LINE_LO_WIDTH + BUG_LINE_HI_WIDTH))' in BUG_FRAME macros more abstract * remove #ifndef BUG_FRAME_STRUCT around BUG_DISP_WIDTH, BUG_LINE_LO_WIDTH as it is required to be defined before as it is used by x86's when the header is included in assembly code. --- Changes in V5: * Remove "#ifdef BUG_FN_REG..." from generic do_bug_frame() as ARM will use generic implementation fully. --- Changes in V4: * common/bug.c: - Use BUG_DEBUGGER_TRAP_FATAL(regs) mnacros instead of debugger_trap_fatal(TRAP_invalid_op, regs) in as TRAP_invalid_op is x86-specific thereby BUG_DEBUGGER_TRAP_FATAL should be defined for each architecture. - add information about what do_bug_frame() returns. - invert the condition 'if ( region )' in do_bug_frame() to reduce the indention. - change type of variable i from 'unsigned int' to 'size_t' as it is compared with n_bugs which has type 'size_t' * xen/bug.h: - Introduce generic BUG_DEBUGGER_TRAP_FATAL(regs) mnacros which is used to deal with debugger_trap_fatal(TRAP_invalid_op, regs) where TRAP_invalid_op is x86-specific - remove '#include ' as it doesn't need any more after switch to x86 implementation. - remove '#include ' as it isn't needed any more - move bug_*() macros inside '#ifndef BUG_FRAME_STRUCT' - add to fix compile issue with BUILD_ON()... - Add documentation for BUG_ASM_CONST. * Update the commit message --- Changes in V3: * Add debugger_trap_fatal() to do_bug_frame(). It simplifies usage of do_bug_frame() for x86 so making handle_bug_frame() and find_bug_frame() not needed anymore. * Update do_bug_frame() to return -EINVAL if something goes wrong; otherwise id of bug_frame * Update _ASM_BUGFRAME_TEXT to make it more portable. * Drop unnecessary comments. * define stub value for TRAP_invalid_op in case if wasn't defined in arch-specific folders. --- Changes in V2: - Switch to x86 implementation as generic as it is more compact ( at least from the point of view of bug frame structure ). - Rename CONFIG_GENERIC_DO_BUG_FRAME to CONFIG_GENERIC_BUG_FRAME. - Change the macro bug_loc(b) to avoid the need for a cast: #define bug_loc(b) ((unsigned long)(b) + (b)->loc_disp) - Rename BUG_FRAME_STUFF to BUG_FRAME_STRUCT - Make macros related to bug frame structure more generic. - Introduce BUG_INSTR and MODIFIER to make _ASM_BUGFRAME_TEXT reusable between x86 and RISC-V. - Rework do_bug_frame() and introduce find_bug_frame() and handle_bug_frame() functions to make it reusable by x86. - code style fixes --- xen/common/Kconfig| 3 + xen/common/Makefile | 1 + xen/common/bug.c | 107 xen/include/xen/bug.h | 162 ++ 4 files changed, 273 insertions(+) create mode 100644 xen/common/bug.c create mode 100644 xen/include/xen/bug.h diff --git a/xen/common/Kconfig b/xen/common/Kconfig index f1ea3199c8..b226323537 100644 --- a/xen/common/Kconfig +++ b/xen/common/Kconfig @@ -28,6 +28,9 @@ config ALTERNATIVE_CALL config ARCH_MAP_DOMAIN_PAGE bool +config GENERIC_BUG_FRAME + bool + config HAS_ALTERNATIVE bool diff --git a/xen/common/Makefile b/xen/common/Makefile index bbd75b4be6..46049eac35 100644 --- a/xen/common/Makefile +++ b/xen/common/Makefile @@ -1,5 +1,6 @@ obj-$(CONFIG_ARGO) += argo.o obj-y += bitmap.o +obj-$(CONFIG_GENERIC_BUG_FRAME) += bug.o obj-$(CONFIG_HYPFS_CONFIG) += config_data.o obj-$(CONFIG_CORE_PARKING) +=
[PATCH v7 3/5] xen: change to
The idea of the patch is to change all to and
keep Xen compilable with adding only minimal amount of changes:
1. It was added "#include " to ARM's "" as it
uses uint_{16,32}t in 'struct bug_frame'.
2. It was added '#define BUG_FRAME_STRUCT' which means that ARM hasn't
been switched to generic implementation yet.
3. It was added '#define BUG_FRAME_STRUCT' which means that x86 hasn't
been switched to generic implementation yet.
4. BUGFRAME_* and _start_bug_frame[], _stop_bug_frame_*[] were removed
for ARM & x86 to deal with compilation errors such as:
redundant redeclaration of ...
5. Remove BUG_DISP_WIDTH, BUG_LINE_LO_WIDTH, BUG_LINE_HI_WIDTH from
x86's to not to produce #undef for them and #define again
with the same values as in . These #undef and #define will
be anyway removed in the patch [2]
In the following two patches x86 and ARM archictectures will be
switched fully:
[1] xen/arm: switch ARM to use generic implementation of bug.h
[2] xen/x86: switch x86 to use generic implemetation of bug.h
Signed-off-by: Oleksii Kurochko
Reviewed-by: Jan Beulich
---
Changes in V7:
* Remove #undef {BUG_DISP_WIDTH, BUG_LINE_LO_WIDTH, BUG_LINE_HI_WIDTH} from
ARM and x86:
* for ARM was created separate patch where the defines are removed as
unused.
* for x86, the defines were removed now not to produce #undef of them to
remove
them again in the following-up patch
* Update commit message
* Add Reviewed-by: Jan Beulich
---
Changes in V6:
- change the inclusion order of .
- add #undef of BUG_DISP_WIDTH, BUG_LINE_LO_WIDTH for ARM & x86
as they were introduced unconditionally in .
- update the commit message
---
Changes in V5:
- Nothing changed
---
Changes in V4:
- defines BUG_DISP_WIDTH, BUG_LINE_LO_WIDTH, BUG_LINE_HI_WIDTH were
moved into
"ifndef BUG_FRAME_STRUCT" in as they are specific for
'struct bug_frame' and so should
co-exist together. So the defines were back to until
BUG_FRAME_STRUCT will be defined in
.
- Update the comment message.
---
Changes in V3:
* Update patch 2 not to break compilation: move some parts from patches 3 and 4
to patch 2:
* move some generic parts from to
* add define BUG_FRAME_STRUCT in ARM's
---
Changes in V2:
* Put [PATCH v1 4/4] xen: change to as second patch,
update the patch to change all to among the whole
project
to not break build.
* Update the commit message.
---
xen/arch/arm/include/asm/bug.h | 17 -
xen/arch/arm/include/asm/div64.h | 2 +-
xen/arch/arm/vgic/vgic-v2.c | 2 +-
xen/arch/arm/vgic/vgic.c | 2 +-
xen/arch/x86/acpi/cpufreq/cpufreq.c | 2 +-
xen/arch/x86/include/asm/asm_defns.h | 2 +-
xen/arch/x86/include/asm/bug.h | 19 ++-
xen/drivers/cpufreq/cpufreq.c| 2 +-
xen/include/xen/lib.h| 2 +-
9 files changed, 13 insertions(+), 37 deletions(-)
diff --git a/xen/arch/arm/include/asm/bug.h b/xen/arch/arm/include/asm/bug.h
index d6c98505bf..cacaf014ab 100644
--- a/xen/arch/arm/include/asm/bug.h
+++ b/xen/arch/arm/include/asm/bug.h
@@ -1,6 +1,8 @@
#ifndef __ARM_BUG_H__
#define __ARM_BUG_H__
+#include
+
#if defined(CONFIG_ARM_32)
# include
#elif defined(CONFIG_ARM_64)
@@ -9,6 +11,8 @@
# error "unknown ARM variant"
#endif
+#define BUG_FRAME_STRUCT
+
struct bug_frame {
signed int loc_disp;/* Relative address to the bug address */
signed int file_disp; /* Relative address to the filename */
@@ -22,13 +26,6 @@ struct bug_frame {
#define bug_line(b) ((b)->line)
#define bug_msg(b) ((const char *)(b) + (b)->msg_disp)
-#define BUGFRAME_run_fn 0
-#define BUGFRAME_warn 1
-#define BUGFRAME_bug2
-#define BUGFRAME_assert 3
-
-#define BUGFRAME_NR 4
-
/* Many versions of GCC doesn't support the asm %c parameter which would
* be preferable to this unpleasantness. We use mergeable string
* sections to avoid multiple copies of the string appearing in the
@@ -85,12 +82,6 @@ struct bug_frame {
unreachable(); \
} while (0)
-extern const struct bug_frame __start_bug_frames[],
- __stop_bug_frames_0[],
- __stop_bug_frames_1[],
- __stop_bug_frames_2[],
- __stop_bug_frames_3[];
-
#endif /* __ARM_BUG_H__ */
/*
* Local variables:
diff --git a/xen/arch/arm/include/asm/div64.h b/xen/arch/arm/include/asm/div64.h
index 1cd58bc51a..fc667a80f9 100644
--- a/xen/arch/arm/include/asm/div64.h
+++ b/xen/arch/arm/include/asm/div64.h
@@ -74,7 +74,7 @@
#elif __GNUC__ >= 4
-#include
+#include
/*
* If the divisor happens to be constant, we determine the appropriate
diff --git a/xen/arch/arm/vgic/vgic-v2.c b/xen/arch/arm/vgic/vgic-v2.c
index 1a99d3a8b4..c90e88fddb 100644
--- a/xen/arch/arm/vgic/vgic-v2.c
+++
[PATCH v7 4/5] xen/arm: switch ARM to use generic implementation of bug.h
The following changes were made:
* make GENERIC_BUG_FRAME mandatory for ARM
* As do_bug_frame() returns -EINVAL in case something goes wrong
otherwise id of bug frame. Thereby 'if' cases where do_bug_frame() was
updated to check if the returned value is less than 0
* Switch ARM's implementation of bug.h macros to generic one
Signed-off-by: Oleksii Kurochko
---
Changes in V7:
* Rebase the patch.
---
Changes in V6:
* Update the "changes in v5"
* Rebase on top of the patch [xen: introduce CONFIG_GENERIC_BUG_FRAME] as
there were minor changes.
---
Changes in V5:
* common/bug.c changes were removed after rebase
(the patch [xen: introduce CONFIG_GENERIC_BUG_FRAME] was reworked to make
ARM implementation to use generic do_bug_frame())
---
Changes in V4:
* Switch ARM implementation to generic one
* Remove BUG_FN_REG from arm{16,32}/bug.h as it isn't needed after switch to
generic implementation
* Update commit message
---
Changes in V3:
* As prototype and what do_bug_frame() returns was changed so patch 3 and 4
was updated to use a new version of do_bug_frame
---
Changes in V2:
* Rename bug_file() in ARM implementation to bug_ptr() as
generic do_bug_frame() uses bug_ptr().
* Remove generic parts from bug.h
* Remove declaration of 'int do_bug_frame(...)'
from as it was introduced in
---
xen/arch/arm/Kconfig | 1 +
xen/arch/arm/arm32/traps.c | 2 +-
xen/arch/arm/include/asm/arm32/bug.h | 2 -
xen/arch/arm/include/asm/arm64/bug.h | 2 -
xen/arch/arm/include/asm/bug.h | 71 +---
xen/arch/arm/include/asm/traps.h | 2 -
xen/arch/arm/traps.c | 81 +---
7 files changed, 4 insertions(+), 157 deletions(-)
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 239d3aed3c..aad6644a7b 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -12,6 +12,7 @@ config ARM_64
config ARM
def_bool y
+ select GENERIC_BUG_FRAME
select HAS_ALTERNATIVE
select HAS_DEVICE_TREE
select HAS_PASSTHROUGH
diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c
index a2fc1c22cb..61c61132c7 100644
--- a/xen/arch/arm/arm32/traps.c
+++ b/xen/arch/arm/arm32/traps.c
@@ -48,7 +48,7 @@ void do_trap_undefined_instruction(struct cpu_user_regs *regs)
if ( instr != BUG_OPCODE )
goto die;
-if ( do_bug_frame(regs, pc) )
+if ( do_bug_frame(regs, pc) < 0 )
goto die;
regs->pc += 4;
diff --git a/xen/arch/arm/include/asm/arm32/bug.h
b/xen/arch/arm/include/asm/arm32/bug.h
index 25cce151dc..3e66f35969 100644
--- a/xen/arch/arm/include/asm/arm32/bug.h
+++ b/xen/arch/arm/include/asm/arm32/bug.h
@@ -10,6 +10,4 @@
#define BUG_INSTR ".word " __stringify(BUG_OPCODE)
-#define BUG_FN_REG r0
-
#endif /* __ARM_ARM32_BUG_H__ */
diff --git a/xen/arch/arm/include/asm/arm64/bug.h
b/xen/arch/arm/include/asm/arm64/bug.h
index 5e11c0dfd5..59f664d7de 100644
--- a/xen/arch/arm/include/asm/arm64/bug.h
+++ b/xen/arch/arm/include/asm/arm64/bug.h
@@ -6,6 +6,4 @@
#define BUG_INSTR "brk " __stringify(BRK_BUG_FRAME_IMM)
-#define BUG_FN_REG x0
-
#endif /* __ARM_ARM64_BUG_H__ */
diff --git a/xen/arch/arm/include/asm/bug.h b/xen/arch/arm/include/asm/bug.h
index cacaf014ab..1d87533044 100644
--- a/xen/arch/arm/include/asm/bug.h
+++ b/xen/arch/arm/include/asm/bug.h
@@ -11,76 +11,7 @@
# error "unknown ARM variant"
#endif
-#define BUG_FRAME_STRUCT
-
-struct bug_frame {
-signed int loc_disp;/* Relative address to the bug address */
-signed int file_disp; /* Relative address to the filename */
-signed int msg_disp;/* Relative address to the predicate (for ASSERT)
*/
-uint16_t line; /* Line number */
-uint32_t pad0:16; /* Padding for 8-bytes align */
-};
-
-#define bug_loc(b) ((const void *)(b) + (b)->loc_disp)
-#define bug_file(b) ((const void *)(b) + (b)->file_disp);
-#define bug_line(b) ((b)->line)
-#define bug_msg(b) ((const char *)(b) + (b)->msg_disp)
-
-/* Many versions of GCC doesn't support the asm %c parameter which would
- * be preferable to this unpleasantness. We use mergeable string
- * sections to avoid multiple copies of the string appearing in the
- * Xen image. BUGFRAME_run_fn needs to be handled separately.
- */
-#define BUG_FRAME(type, line, file, has_msg, msg) do { \
-BUILD_BUG_ON((line) >> 16); \
-BUILD_BUG_ON((type) >= BUGFRAME_NR);\
-asm ("1:"BUG_INSTR"\n" \
- ".pushsection .rodata.str, \"aMS\", %progbits, 1\n"\
- "2:\t.asciz " __stringify(file) "\n" \
- "3:\n" \
- ".if " #has_msg "\n" \
- "\t.asciz " #msg "\n"
[PATCH v7 5/5] xen/x86: switch x86 to use generic implemetation of bug.h
The following changes were made:
* Make GENERIC_BUG_FRAME mandatory for X86
* Update asm/bug.h using generic implementation in
* Update do_invalid_op using generic do_bug_frame()
* Define BUG_DEBUGGER_TRAP_FATAL to debugger_trap_fatal(X86_EXC_GP,regs)
* type of eip variable was changed to 'const void *'
* add '#include '
Signed-off-by: Oleksii Kurochko
Reviewed-by: Jan Beulich
---
Changes in V7:
* update the commit message
* make eip 'const void *'
* change [eip = (unsigned char *)eip + sizeof(bug_insn);] to [eip +=
sizeof(bug_insn);]
* add '#include ' to
* add Reviewed-by: Jan Beulich
---
Changes in V6:
* update the commit message
* update the type of eip to 'void *' in do_invalid_op()
* fix the logic of do_invalid_op()
* move macros BUG_DEBUGGER_TRAP_FATAL under #ifndef __ASSEMBLY__ as
it is not necessary to be in assembly code.
---
Changes in V5:
* Nothing changed
---
Changes in V4:
* Back comment /* !__ASSEMBLY__ */ for #else case in
* Remove changes related to x86/.../asm/debuger.h as do_bug_frame() prototype
was updated and cpu_user_regs isn't const any more.
---
Changes in V3:
* As prototype and what do_bug_frame() returns was changed so patch 3 and 4
was updated to use a new version of do_bug_frame
* MODIFIER was change to BUG_ASM_CONST to align with generic implementation
---
Changes in V2:
* Remove all unnecessary things from as they were introduced in
.
* Define BUG_INSTR = 'ud2' and MODIFIER = 'c' ( it is needed to skip '$'
when use an imidiate in x86 assembly )
* Update do_invalid_op() to re-use handle_bug_frame() and find_bug_frame()
from generic implemetation of CONFIG_GENERIC_BUG_FRAME
* Code style fixes.
---
xen/arch/x86/Kconfig | 1 +
xen/arch/x86/include/asm/bug.h | 69 ++---
xen/arch/x86/traps.c | 81 --
3 files changed, 13 insertions(+), 138 deletions(-)
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 6a7825f4ba..b0ff1f3ee6 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -11,6 +11,7 @@ config X86
select ARCH_MAP_DOMAIN_PAGE
select ARCH_SUPPORTS_INT128
select CORE_PARKING
+ select GENERIC_BUG_FRAME
select HAS_ALTERNATIVE
select HAS_COMPAT
select HAS_CPUFREQ
diff --git a/xen/arch/x86/include/asm/bug.h b/xen/arch/x86/include/asm/bug.h
index 4b3e7b019d..8531d5f91e 100644
--- a/xen/arch/x86/include/asm/bug.h
+++ b/xen/arch/x86/include/asm/bug.h
@@ -3,73 +3,12 @@
#ifndef __ASSEMBLY__
-#define BUG_FRAME_STRUCT
+#include
-struct bug_frame {
-signed int loc_disp:BUG_DISP_WIDTH;
-unsigned int line_hi:BUG_LINE_HI_WIDTH;
-signed int ptr_disp:BUG_DISP_WIDTH;
-unsigned int line_lo:BUG_LINE_LO_WIDTH;
-signed int msg_disp[];
-};
+#define BUG_DEBUGGER_TRAP_FATAL(regs) debugger_trap_fatal(X86_EXC_GP,regs)
-#define bug_loc(b) ((const void *)(b) + (b)->loc_disp)
-#define bug_ptr(b) ((const void *)(b) + (b)->ptr_disp)
-#define bug_line(b) (b)->line_hi + ((b)->loc_disp < 0)) &\
- ((1 << BUG_LINE_HI_WIDTH) - 1)) <<\
- BUG_LINE_LO_WIDTH) + \
- (((b)->line_lo + ((b)->ptr_disp < 0)) & \
- ((1 << BUG_LINE_LO_WIDTH) - 1)))
-#define bug_msg(b) ((const char *)(b) + (b)->msg_disp[1])
-
-#define _ASM_BUGFRAME_TEXT(second_frame) \
-".Lbug%=: ud2\n" \
-".pushsection .bug_frames.%c[bf_type], \"a\", @progbits\n" \
-".p2align 2\n" \
-".Lfrm%=:\n" \
-".long (.Lbug%= - .Lfrm%=) + %c[bf_line_hi]\n" \
-".long (%c[bf_ptr] - .Lfrm%=) + %c[bf_line_lo]\n"\
-".if " #second_frame "\n"\
-".long 0, %c[bf_msg] - .Lfrm%=\n"\
-".endif\n" \
-".popsection\n" \
-
-#define _ASM_BUGFRAME_INFO(type, line, ptr, msg) \
-[bf_type]"i" (type), \
-[bf_ptr] "i" (ptr), \
-[bf_msg] "i" (msg), \
-[bf_line_lo] "i" ((line & ((1 << BUG_LINE_LO_WIDTH) - 1))\
- << BUG_DISP_WIDTH),\
-[bf_line_hi] "i" (((line) >> BUG_LINE_LO_WIDTH) << BUG_DISP_WIDTH)
-
-#define BUG_FRAME(type, line, ptr, second_frame, msg) do { \
-BUILD_BUG_ON((line) >>
[PATCH v7 0/5] introduce generic implementation of macros from bug.h
A large part of the content of the bug.h is repeated among all
architectures (especially x86 and RISCV have the same implementation), so it
was created a new config CONFIG_GENERIC_BUG_FRAME which is used to
introduce generic implementation of do_bug_frame() and move x86's
to with the following changes:
* Add inclusion of arch-specific header
* Rename the guard and remove x86 specific changes
* Wrap macros BUG_FRAME/run_in_exception_handler/WARN/BUG/assert_failed/etc
into #ifndef "BUG_FRAME/run_in_exception_handler/WARN/BUG/assert_failed/etc"
thereby each architecture can override the generic implementation of macros.
* Add #if{n}def __ASSEMBLY__ ... #endif
* Introduce BUG_FRAME_STRUCTURE define to be able to change the structure of
bug
frame
* Introduce BUG_INSTR and BUG_ASM_CONST to make _ASM_BUGFRAME_TEXT reusable
between
architectures
* Introduce BUG_DEBUGGER_TRAP_FATAL to hide details about TRAP_invalid_op for
specific
architecture
* Introduce BUILD_BUG_ON_LINE_WIDTH(line) to make more generic BUG_FRAME
macros
* Make macros related to bug frame structure more generic.
RISC-V will be switched to use and in the future, it will use common
the version of do_bug_frame() when xen/common will work for RISC-V.
---
Changes in V7:
* Introduce new patch to clean up ARM's from unused defines:
BUG_DISP_WIDTH, BUG_LINE_LO_WIDTH, BUG_LINE_HI_WIDTH
* fix addressed code style.
* Remove '#include ' from bug.c. it should be a part
of .
* move BUILD_BUG_ON_LINE_WIDTH to '#ifndef BUG_FRAME_STRUCT' and define
dummy BUILD_BUG_ON_LINE_WIDTH when BUG_FRAME_STRUCT is defined.
* remove update prototype of 'void (*fn)(const struct cpu_user_regs *)' to
'void (*fn)(struct cpu_user_regs *)'.
* add code to to make sure the type used in run_in_exception_handler()
matches the one used here.
* Remove #undef {BUG_DISP_WIDTH, BUG_LINE_LO_WIDTH, BUG_LINE_HI_WIDTH} from
ARM and x86:
- for ARM was created separate patch where the defines are removed as
unused.
- for x86, the defines were removed now not to produce #undef of them
to remove
them again in the following-up patch
* make eip 'const void *' in x86's do_invalid_op()
* change [eip = (unsigned char *)eip + sizeof(bug_insn);] to [eip +=
sizeof(bug_insn);]
* add '#include ' to x86's
---
Changes in V6:
* Update the cover letter message: add information that
BUG_DEBUGGER_TRAP_FATAL() and
BUILD_BUG_ON_LINE_WIDTH().
* Introduce BUILD_BUG_ON_LINE_WIDTH(line) to make more generic BUG_FRAME
macros.
* fix addressed code style
* change -EINVAL to -ENOENT in case when bug_frame wasn't found in generic
do_bug_frame().
* change all 'return id' to 'break' inside switch/case of do_bug_frame().
* move up "#ifndef __ASSEMBLY__" to include BUG_DEBUGGER_TRAP_FATAL.
* update the comment of BUG_ASM_CONST.
* Introduce BUILD_BUG_ON_LINE_WIDTH(line) to make more generic BUG_FRAME macros
* remove #ifndef BUG_FRAME_STRUCT around BUG_DISP_WIDTH, BUG_LINE_LO_WIDTH as
it is
required to be defined before as it is used by x86's
when
the header is included in assembly code.
* add undef of BUG_DISP_WIDTH, BUG_LINE_LO_WIDTH for ARM & x86
as they were introduced unconditionally in .
* update the type of eip to 'void *' in do_invalid_op() for x86
* fix the logic of do_invalid_op() for x86
* move macros BUG_DEBUGGER_TRAP_FATAL under #ifndef __ASSEMBLY__ as
it is not necessary to be in assembly code for x86.
---
Changes in V5:
* Update the cover letter message as the patch, on which the patch series
is based, has been merged to staging.
* Remove "#ifdef BUG_FN_REG..." from generic do_bug_frame() as ARM will
use generic implementation fully.
---
Changes in V4:
* Introduce and use generic BUG_DEBUGGER_TRAP_FATAL(regs) mnacros which is
used to deal with
debugger_trap_fatal(TRAP_invalid_op, regs) where TRAP_invalid_op is
x86-specific.
* Add comment what do_bug_frame() returns.
* Do refactoring of do_bug_frame():
* invert the condition 'if ( region )' in do_bug_frame() to reduce the
indention.
* change type of variable i from 'unsigned int' to 'size_t' as
it is compared with
n_bugs which has type 'size_t'
* Remove '#include ' from as it doesn't need any
more after switch to
x86 implementation.
* Remove '#include ' as it isn't needed any more
* Move bug_*() macros inside '#ifndef BUG_FRAME_STRUCT'
* Add to fix compile issue with BUILD_ON()...
* Add documentation for BUG_ASM_CONST.
* Defines BUG_DISP_WIDTH, BUG_LINE_LO_WIDTH, BUG_LINE_HI_WIDTH were moved into
"ifndef BUG_FRAME_STRUCT" in as they are specific for 'struct
bug_frame' and so should
co-exist together. So the defines were back to until
BUG_FRAME_STRUCT will be defined in
.
* Switch ARM implementation to generic one
* Remove BUG_FN_REG from arm{16,32}/bug.h as it isn't needed after switch to
[PATCH v7 2/5] xen/arm: remove unused defines in
The following defines BUG_DISP_WIDTH, BUG_LINE_LO_WIDTH,
BUG_LINE_HI_WIDTH aren't used in ARM so could be purged
as unused.
Signed-off-by: Oleksii Kurochko
---
xen/arch/arm/include/asm/bug.h | 4
1 file changed, 4 deletions(-)
diff --git a/xen/arch/arm/include/asm/bug.h b/xen/arch/arm/include/asm/bug.h
index f4088d0913..d6c98505bf 100644
--- a/xen/arch/arm/include/asm/bug.h
+++ b/xen/arch/arm/include/asm/bug.h
@@ -9,10 +9,6 @@
# error "unknown ARM variant"
#endif
-#define BUG_DISP_WIDTH24
-#define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH)
-#define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH)
-
struct bug_frame {
signed int loc_disp;/* Relative address to the bug address */
signed int file_disp; /* Relative address to the filename */
--
2.39.2
[ovmf test] 179517: all pass - PUSHED
flight 179517 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/179517/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf a0f9628705e35c981ae95376f9ebedf877d09111 baseline version: ovmf 69da506c927f8092ea8f783a092a694a3582e3ef Last test of basis 179510 2023-03-08 18:43:52 Z0 days Testing same since 179517 2023-03-09 10:12:20 Z0 days1 attempts People who touched revisions under test: Gerd Hoffmann jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git 69da506c92..a0f9628705 a0f9628705e35c981ae95376f9ebedf877d09111 -> xen-tested-master
Re: [PATCH v1 2/2] arch/arm: time: Add support for parsing interrupts by names
Hi Michal,
> On 9 Mar 2023, at 13:42, Michal Orzel wrote:
>
> Hi Bertrand,
>
> On 09/03/2023 13:19, Bertrand Marquis wrote:
>>
>>
>> Hi Michal,
>>
>>> On 9 Mar 2023, at 12:35, Michal Orzel wrote:
>>>
>>>
>>>
>>> On 09/03/2023 11:39, Bertrand Marquis wrote:
Hi Michal,
> On 9 Mar 2023, at 11:05, Michal Orzel wrote:
>
>
>
> On 09/03/2023 09:02, Bertrand Marquis wrote:
>>
>>
>> Hi Stefano,
>>
>>> On 7 Mar 2023, at 22:02, Stefano Stabellini
>>> wrote:
>>>
>>> On Tue, 7 Mar 2023, Bertrand Marquis wrote:
> On 7 Mar 2023, at 11:09, Andrei Cherechesu (OSS)
> wrote:
>
> From: Andrei Cherechesu
>
> Added support for parsing the ARM generic timer interrupts DT
> node by the "interrupt-names" property, if it is available.
>
> If not available, the usual parsing based on the expected
> IRQ order is performed.
>
> Also added the "hyp-virt" PPI to the timer PPI list, even
> though it's currently not in use. If the "hyp-virt" PPI is
> not found, the hypervisor won't panic.
>
> Signed-off-by: Andrei Cherechesu
> ---
> xen/arch/arm/include/asm/time.h | 3 ++-
> xen/arch/arm/time.c | 26 ++
> 2 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/time.h
> b/xen/arch/arm/include/asm/time.h
> index 4b401c1110..49ad8c1a6d 100644
> --- a/xen/arch/arm/include/asm/time.h
> +++ b/xen/arch/arm/include/asm/time.h
> @@ -82,7 +82,8 @@ enum timer_ppi
> TIMER_PHYS_NONSECURE_PPI = 1,
> TIMER_VIRT_PPI = 2,
> TIMER_HYP_PPI = 3,
> -MAX_TIMER_PPI = 4,
> +TIMER_HYP_VIRT_PPI = 4,
> +MAX_TIMER_PPI = 5,
> };
>
> /*
> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
> index 433d7be909..794da646d6 100644
> --- a/xen/arch/arm/time.c
> +++ b/xen/arch/arm/time.c
> @@ -38,6 +38,14 @@ uint32_t __read_mostly timer_dt_clock_frequency;
>
> static unsigned int timer_irq[MAX_TIMER_PPI];
>
> +static const char *timer_irq_names[MAX_TIMER_PPI] = {
> +[TIMER_PHYS_SECURE_PPI] = "sec-phys",
> +[TIMER_PHYS_NONSECURE_PPI] = "phys",
> +[TIMER_VIRT_PPI] = "virt",
> +[TIMER_HYP_PPI] = "hyp-phys",
> +[TIMER_HYP_VIRT_PPI] = "hyp-virt",
> +};
> +
I would need some reference or a pointer to some doc to check those.
> unsigned int timer_get_irq(enum timer_ppi ppi)
> {
> ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI);
> @@ -149,15 +157,25 @@ static void __init init_dt_xen_time(void)
> {
> int res;
> unsigned int i;
> +bool has_names;
> +
> +has_names = dt_property_read_bool(timer, "interrupt-names");
>
> /* Retrieve all IRQs for the timer */
> for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
> {
> -res = platform_get_irq(timer, i);
> -
> -if ( res < 0 )
> +if ( has_names )
> +res = platform_get_irq_byname(timer, timer_irq_names[i]);
> +else
> +res = platform_get_irq(timer, i);
> +
> +if ( res > 0 )
The behaviour of the code is changed here compared to the current
version as res = 0 will now generate a panic.
Some device tree might not specify an interrupt number and just put
0 and Xen will now panic on those systems.
As I have no idea if such systems exists and the behaviour is modified
you should justify this and mention it in the commit message or keep
the old behaviour and let 0 go through without a panic.
@stefano, julien any idea here ? should just keep the old behaviour ?
>>>
>>> platform_get_irq returns 0 if the irq is 0. The irq cannot be 0 because
>>> 0 is reserved for SGIs, not PPIs. So I think it is OK to consider 0 an
>>> error.
>>
>> Problem here is that a DTB might not specify all interrupts and just put
>> 0 for the one not used (or not available for example if you have no
>> secure
>> world).
> Xen requires presence of EL3,EL2 and on such system, at least the
> following timers needs to be there
> according to Arm ARM:
> - EL3 phys (if EL3 is there)
This might be needed by EL3 but not by Xen.
>>> Xen requires system with EL3 and if there is EL3, both Arm spec and dt
>>> bindings requires sec-phys timer to be there.
>>> So it would be very strange to see a fake
Re: [PATCH v1 2/2] arch/arm: time: Add support for parsing interrupts by names
Hi Bertrand,
On 09/03/2023 13:19, Bertrand Marquis wrote:
>
>
> Hi Michal,
>
>> On 9 Mar 2023, at 12:35, Michal Orzel wrote:
>>
>>
>>
>> On 09/03/2023 11:39, Bertrand Marquis wrote:
>>>
>>>
>>> Hi Michal,
>>>
On 9 Mar 2023, at 11:05, Michal Orzel wrote:
On 09/03/2023 09:02, Bertrand Marquis wrote:
>
>
> Hi Stefano,
>
>> On 7 Mar 2023, at 22:02, Stefano Stabellini
>> wrote:
>>
>> On Tue, 7 Mar 2023, Bertrand Marquis wrote:
On 7 Mar 2023, at 11:09, Andrei Cherechesu (OSS)
wrote:
From: Andrei Cherechesu
Added support for parsing the ARM generic timer interrupts DT
node by the "interrupt-names" property, if it is available.
If not available, the usual parsing based on the expected
IRQ order is performed.
Also added the "hyp-virt" PPI to the timer PPI list, even
though it's currently not in use. If the "hyp-virt" PPI is
not found, the hypervisor won't panic.
Signed-off-by: Andrei Cherechesu
---
xen/arch/arm/include/asm/time.h | 3 ++-
xen/arch/arm/time.c | 26 ++
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/xen/arch/arm/include/asm/time.h
b/xen/arch/arm/include/asm/time.h
index 4b401c1110..49ad8c1a6d 100644
--- a/xen/arch/arm/include/asm/time.h
+++ b/xen/arch/arm/include/asm/time.h
@@ -82,7 +82,8 @@ enum timer_ppi
TIMER_PHYS_NONSECURE_PPI = 1,
TIMER_VIRT_PPI = 2,
TIMER_HYP_PPI = 3,
-MAX_TIMER_PPI = 4,
+TIMER_HYP_VIRT_PPI = 4,
+MAX_TIMER_PPI = 5,
};
/*
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 433d7be909..794da646d6 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -38,6 +38,14 @@ uint32_t __read_mostly timer_dt_clock_frequency;
static unsigned int timer_irq[MAX_TIMER_PPI];
+static const char *timer_irq_names[MAX_TIMER_PPI] = {
+[TIMER_PHYS_SECURE_PPI] = "sec-phys",
+[TIMER_PHYS_NONSECURE_PPI] = "phys",
+[TIMER_VIRT_PPI] = "virt",
+[TIMER_HYP_PPI] = "hyp-phys",
+[TIMER_HYP_VIRT_PPI] = "hyp-virt",
+};
+
>>>
>>> I would need some reference or a pointer to some doc to check those.
>>>
unsigned int timer_get_irq(enum timer_ppi ppi)
{
ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI);
@@ -149,15 +157,25 @@ static void __init init_dt_xen_time(void)
{
int res;
unsigned int i;
+bool has_names;
+
+has_names = dt_property_read_bool(timer, "interrupt-names");
/* Retrieve all IRQs for the timer */
for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
{
-res = platform_get_irq(timer, i);
-
-if ( res < 0 )
+if ( has_names )
+res = platform_get_irq_byname(timer, timer_irq_names[i]);
+else
+res = platform_get_irq(timer, i);
+
+if ( res > 0 )
>>>
>>> The behaviour of the code is changed here compared to the current
>>> version as res = 0 will now generate a panic.
>>>
>>> Some device tree might not specify an interrupt number and just put
>>> 0 and Xen will now panic on those systems.
>>> As I have no idea if such systems exists and the behaviour is modified
>>> you should justify this and mention it in the commit message or keep
>>> the old behaviour and let 0 go through without a panic.
>>>
>>> @stefano, julien any idea here ? should just keep the old behaviour ?
>>
>> platform_get_irq returns 0 if the irq is 0. The irq cannot be 0 because
>> 0 is reserved for SGIs, not PPIs. So I think it is OK to consider 0 an
>> error.
>
> Problem here is that a DTB might not specify all interrupts and just put
> 0 for the one not used (or not available for example if you have no secure
> world).
Xen requires presence of EL3,EL2 and on such system, at least the
following timers needs to be there
according to Arm ARM:
- EL3 phys (if EL3 is there)
>>>
>>> This might be needed by EL3 but not by Xen.
>> Xen requires system with EL3 and if there is EL3, both Arm spec and dt
>> bindings requires sec-phys timer to be there.
>> So it would be very strange to see a fake interrupt with IRQ being 0. But if
>> we relly want to only care about
>> what Xen needs, then we could live with that (although it is difficult for
>> me to find justification for 0 there).
>> Device trees are created per system and
Re: [PATCH v1 2/2] arch/arm: time: Add support for parsing interrupts by names
Hi Michal,
> On 9 Mar 2023, at 12:35, Michal Orzel wrote:
>
>
>
> On 09/03/2023 11:39, Bertrand Marquis wrote:
>>
>>
>> Hi Michal,
>>
>>> On 9 Mar 2023, at 11:05, Michal Orzel wrote:
>>>
>>>
>>>
>>> On 09/03/2023 09:02, Bertrand Marquis wrote:
Hi Stefano,
> On 7 Mar 2023, at 22:02, Stefano Stabellini
> wrote:
>
> On Tue, 7 Mar 2023, Bertrand Marquis wrote:
>>> On 7 Mar 2023, at 11:09, Andrei Cherechesu (OSS)
>>> wrote:
>>>
>>> From: Andrei Cherechesu
>>>
>>> Added support for parsing the ARM generic timer interrupts DT
>>> node by the "interrupt-names" property, if it is available.
>>>
>>> If not available, the usual parsing based on the expected
>>> IRQ order is performed.
>>>
>>> Also added the "hyp-virt" PPI to the timer PPI list, even
>>> though it's currently not in use. If the "hyp-virt" PPI is
>>> not found, the hypervisor won't panic.
>>>
>>> Signed-off-by: Andrei Cherechesu
>>> ---
>>> xen/arch/arm/include/asm/time.h | 3 ++-
>>> xen/arch/arm/time.c | 26 ++
>>> 2 files changed, 24 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/include/asm/time.h
>>> b/xen/arch/arm/include/asm/time.h
>>> index 4b401c1110..49ad8c1a6d 100644
>>> --- a/xen/arch/arm/include/asm/time.h
>>> +++ b/xen/arch/arm/include/asm/time.h
>>> @@ -82,7 +82,8 @@ enum timer_ppi
>>> TIMER_PHYS_NONSECURE_PPI = 1,
>>> TIMER_VIRT_PPI = 2,
>>> TIMER_HYP_PPI = 3,
>>> -MAX_TIMER_PPI = 4,
>>> +TIMER_HYP_VIRT_PPI = 4,
>>> +MAX_TIMER_PPI = 5,
>>> };
>>>
>>> /*
>>> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
>>> index 433d7be909..794da646d6 100644
>>> --- a/xen/arch/arm/time.c
>>> +++ b/xen/arch/arm/time.c
>>> @@ -38,6 +38,14 @@ uint32_t __read_mostly timer_dt_clock_frequency;
>>>
>>> static unsigned int timer_irq[MAX_TIMER_PPI];
>>>
>>> +static const char *timer_irq_names[MAX_TIMER_PPI] = {
>>> +[TIMER_PHYS_SECURE_PPI] = "sec-phys",
>>> +[TIMER_PHYS_NONSECURE_PPI] = "phys",
>>> +[TIMER_VIRT_PPI] = "virt",
>>> +[TIMER_HYP_PPI] = "hyp-phys",
>>> +[TIMER_HYP_VIRT_PPI] = "hyp-virt",
>>> +};
>>> +
>>
>> I would need some reference or a pointer to some doc to check those.
>>
>>> unsigned int timer_get_irq(enum timer_ppi ppi)
>>> {
>>> ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI);
>>> @@ -149,15 +157,25 @@ static void __init init_dt_xen_time(void)
>>> {
>>> int res;
>>> unsigned int i;
>>> +bool has_names;
>>> +
>>> +has_names = dt_property_read_bool(timer, "interrupt-names");
>>>
>>> /* Retrieve all IRQs for the timer */
>>> for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
>>> {
>>> -res = platform_get_irq(timer, i);
>>> -
>>> -if ( res < 0 )
>>> +if ( has_names )
>>> +res = platform_get_irq_byname(timer, timer_irq_names[i]);
>>> +else
>>> +res = platform_get_irq(timer, i);
>>> +
>>> +if ( res > 0 )
>>
>> The behaviour of the code is changed here compared to the current
>> version as res = 0 will now generate a panic.
>>
>> Some device tree might not specify an interrupt number and just put
>> 0 and Xen will now panic on those systems.
>> As I have no idea if such systems exists and the behaviour is modified
>> you should justify this and mention it in the commit message or keep
>> the old behaviour and let 0 go through without a panic.
>>
>> @stefano, julien any idea here ? should just keep the old behaviour ?
>
> platform_get_irq returns 0 if the irq is 0. The irq cannot be 0 because
> 0 is reserved for SGIs, not PPIs. So I think it is OK to consider 0 an
> error.
Problem here is that a DTB might not specify all interrupts and just put
0 for the one not used (or not available for example if you have no secure
world).
>>> Xen requires presence of EL3,EL2 and on such system, at least the following
>>> timers needs to be there
>>> according to Arm ARM:
>>> - EL3 phys (if EL3 is there)
>>
>> This might be needed by EL3 but not by Xen.
> Xen requires system with EL3 and if there is EL3, both Arm spec and dt
> bindings requires sec-phys timer to be there.
> So it would be very strange to see a fake interrupt with IRQ being 0. But if
> we relly want to only care about
> what Xen needs, then we could live with that (although it is difficult for me
> to find justification for 0 there).
> Device trees are created per system and if system has EL3, then why forcing 0
> to be listed for sec-phys timer?
>
Let's see that on the other angle: why should Xen check stuff that it does not
need ?
Re: API/ABIs: Re: [RFC PATCH v2 0/2] Add a new acquire resource to query vcpu statistics
On 09.03.2023 11:38, Matias Ezequiel Vara Larsen wrote:
> On Wed, Mar 08, 2023 at 03:16:05PM +0100, Jan Beulich wrote:
>> On 08.03.2023 12:54, Matias Ezequiel Vara Larsen wrote:
>>> On Tue, Mar 07, 2023 at 11:12:00AM +0100, Jan Beulich wrote:
On 06.03.2023 15:23, Matias Ezequiel Vara Larsen wrote:
> - Xen shall use the "stats_active" field to indicate what it is
> producing. In
> this field, reserved bits shall be 0. This shall allow us to agree on
> the
> layout even when producer and consumer are compiled with different
> headers.
I wonder how well such a bitfield is going to scale. It provides for
only 32 (maybe 64) counters. Of course this may seem a lot right now,
but you never know how quickly something like this can grow. Plus
with ...
>>>
>>> Would it make sense to define it like this?:
>>>
>>> struct vcpu_shmem_stats {
>>> #define STATS_A (1u << 0)
>>> ...
>>> #define VCPU_STATS_MAGIC 0xaabbccdd
>>> uint32_t magic;
>>> uint32_t offset; // roundup(sizeof(vcpu_shmem_stats) +
>>> sizeof(uint32_t) * nr_stats, cacheline_size)
>>> uint32_t size;// sizeof(vcpu_stats)
>>> uint32_t stride; // roundup(sizeof(vcpu_stats), cacheline_size)
>>> uint32_t nr_stats; // size of stats_active in uint32_t
>>> uint32_t stats_active[XEN_FLEX_ARRAY_DIM];
>>> ...
>>> };
>>
>
> The use of stats_active[] is meant to have a bitmap that could scale thus not
> limiting the number of counters in the vcpu_stat structure to 32 or 64. I
> can't
> see other way to have an unlimited number of counters though.
>
>> Possibly, but this would make it harder to use the interface. An alternative
>> might be to specify that an actual stats value set to ~0 marks an inactive
>> element. Since these are 64-bit counters, with today's and tomorrow's
>> computers we won't be at risk of a counter reaching a value of 2^^64-1, I
>> guess. And even if there was one where such a risk could not be excluded
>> (e.g. because of using pretty large increments), one would merely need to
>> make sure to saturate at 2^^64-2. Plus at such a point one would need to
>> consider anyway to switch to 128-bit fields, as neither saturated nor
>> wrapped values are of much use to consumers.
>>
>
> If I understand well, this use-case is in case an element in the stats_active
> bitmap becomes inactive, i.e., it is set to "0" in stats_active[]. You are
> proposing to set to ~0 the actual stats value to mark an inactive element. I
> may be missing something here but would not be enough to set to "0" the
> corresponding stats_active[] bit?
The suggestion was to eliminate the need for stats_active[].
Jan
Re: [PATCH v1 2/2] arch/arm: time: Add support for parsing interrupts by names
On 09/03/2023 11:39, Bertrand Marquis wrote:
>
>
> Hi Michal,
>
>> On 9 Mar 2023, at 11:05, Michal Orzel wrote:
>>
>>
>>
>> On 09/03/2023 09:02, Bertrand Marquis wrote:
>>>
>>>
>>> Hi Stefano,
>>>
On 7 Mar 2023, at 22:02, Stefano Stabellini wrote:
On Tue, 7 Mar 2023, Bertrand Marquis wrote:
>> On 7 Mar 2023, at 11:09, Andrei Cherechesu (OSS)
>> wrote:
>>
>> From: Andrei Cherechesu
>>
>> Added support for parsing the ARM generic timer interrupts DT
>> node by the "interrupt-names" property, if it is available.
>>
>> If not available, the usual parsing based on the expected
>> IRQ order is performed.
>>
>> Also added the "hyp-virt" PPI to the timer PPI list, even
>> though it's currently not in use. If the "hyp-virt" PPI is
>> not found, the hypervisor won't panic.
>>
>> Signed-off-by: Andrei Cherechesu
>> ---
>> xen/arch/arm/include/asm/time.h | 3 ++-
>> xen/arch/arm/time.c | 26 ++
>> 2 files changed, 24 insertions(+), 5 deletions(-)
>>
>> diff --git a/xen/arch/arm/include/asm/time.h
>> b/xen/arch/arm/include/asm/time.h
>> index 4b401c1110..49ad8c1a6d 100644
>> --- a/xen/arch/arm/include/asm/time.h
>> +++ b/xen/arch/arm/include/asm/time.h
>> @@ -82,7 +82,8 @@ enum timer_ppi
>> TIMER_PHYS_NONSECURE_PPI = 1,
>> TIMER_VIRT_PPI = 2,
>> TIMER_HYP_PPI = 3,
>> -MAX_TIMER_PPI = 4,
>> +TIMER_HYP_VIRT_PPI = 4,
>> +MAX_TIMER_PPI = 5,
>> };
>>
>> /*
>> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
>> index 433d7be909..794da646d6 100644
>> --- a/xen/arch/arm/time.c
>> +++ b/xen/arch/arm/time.c
>> @@ -38,6 +38,14 @@ uint32_t __read_mostly timer_dt_clock_frequency;
>>
>> static unsigned int timer_irq[MAX_TIMER_PPI];
>>
>> +static const char *timer_irq_names[MAX_TIMER_PPI] = {
>> +[TIMER_PHYS_SECURE_PPI] = "sec-phys",
>> +[TIMER_PHYS_NONSECURE_PPI] = "phys",
>> +[TIMER_VIRT_PPI] = "virt",
>> +[TIMER_HYP_PPI] = "hyp-phys",
>> +[TIMER_HYP_VIRT_PPI] = "hyp-virt",
>> +};
>> +
>
> I would need some reference or a pointer to some doc to check those.
>
>> unsigned int timer_get_irq(enum timer_ppi ppi)
>> {
>> ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI);
>> @@ -149,15 +157,25 @@ static void __init init_dt_xen_time(void)
>> {
>> int res;
>> unsigned int i;
>> +bool has_names;
>> +
>> +has_names = dt_property_read_bool(timer, "interrupt-names");
>>
>> /* Retrieve all IRQs for the timer */
>> for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
>> {
>> -res = platform_get_irq(timer, i);
>> -
>> -if ( res < 0 )
>> +if ( has_names )
>> +res = platform_get_irq_byname(timer, timer_irq_names[i]);
>> +else
>> +res = platform_get_irq(timer, i);
>> +
>> +if ( res > 0 )
>
> The behaviour of the code is changed here compared to the current
> version as res = 0 will now generate a panic.
>
> Some device tree might not specify an interrupt number and just put
> 0 and Xen will now panic on those systems.
> As I have no idea if such systems exists and the behaviour is modified
> you should justify this and mention it in the commit message or keep
> the old behaviour and let 0 go through without a panic.
>
> @stefano, julien any idea here ? should just keep the old behaviour ?
platform_get_irq returns 0 if the irq is 0. The irq cannot be 0 because
0 is reserved for SGIs, not PPIs. So I think it is OK to consider 0 an
error.
>>>
>>> Problem here is that a DTB might not specify all interrupts and just put
>>> 0 for the one not used (or not available for example if you have no secure
>>> world).
>> Xen requires presence of EL3,EL2 and on such system, at least the following
>> timers needs to be there
>> according to Arm ARM:
>> - EL3 phys (if EL3 is there)
>
> This might be needed by EL3 but not by Xen.
Xen requires system with EL3 and if there is EL3, both Arm spec and dt bindings
requires sec-phys timer to be there.
So it would be very strange to see a fake interrupt with IRQ being 0. But if we
relly want to only care about
what Xen needs, then we could live with that (although it is difficult for me
to find justification for 0 there).
Device trees are created per system and if system has EL3, then why forcing 0
to be listed for sec-phys timer?
~Michal
Re: [SeaBIOS] Re: [SeaBIOS PATCH] xen: require Xen info structure at 0x1000 to detect Xen
On Tue, Mar 07, 2023 at 08:42:18AM +, David Woodhouse wrote: > On Thu, 2023-02-02 at 10:10 +0100, Gerd Hoffmann wrote: > > > Thanks, Kevin. > > > > > > I'd like to get the rest of the Xen platform support in to qemu 8.0 > > > if > > > possible. Which is currently scheduled for March. > > > > > > Is there likely to be a SeaBIOS release before then which Gerd > > > would > > > pull into qemu anyway, or should I submit a submodule update to a > > > snapshot of today's tree? That would just pull in this commit, and > > > the > > > one other fix that's in the SeaBIOS tree since 1.16.1? > > > > Tagging 1.16.2 in time for the qemu 8.0 should not be a problem given > > that we have only bugfixes in master. Roughly around soft freeze is > > probably a good time for that. > > That's, erm, at the *end* of today 2023-03-07, and the freeze happens > only *after* Paul has reviewed the phase 2 Xen PV back end support, > right? Ok, we have as of today two changes: kraxel@sirius ~/projects/seabios (master)# git log --oneline rel-1.16.1.. ea1b7a073390 xen: require Xen info structure at 0x1000 to detect Xen 645a64b4911d usb: fix wrong init of keyboard/mouse's if first interface is not boot protocol With no changes since January and no known issues. I think we can safely tag the current state as 1.16.2. I'll do that next monday (plus qemu pull request) unless there are objections until then. take care, Gerd
Re: [PATCH v2] hvc/xen: prevent concurrent accesses to the shared ring
Hello, It's been 3 months and no reply. On Mon, Dec 12, 2022 at 01:36:48PM +0100, Roger Pau Monné wrote: > Hello, > > Gentle ping regarding the locking question below. > > Thanks, Roger. > > On Fri, Dec 02, 2022 at 12:40:05PM +0100, Roger Pau Monné wrote: > > On Wed, Nov 30, 2022 at 05:08:06PM -0800, Stefano Stabellini wrote: > > > On Wed, 30 Nov 2022, Roger Pau Monne wrote: > > > > The hvc machinery registers both a console and a tty device based on > > > > the hv ops provided by the specific implementation. Those two > > > > interfaces however have different locks, and there's no single locks > > > > that's shared between the tty and the console implementations, hence > > > > the driver needs to protect itself against concurrent accesses. > > > > Otherwise concurrent calls using the split interfaces are likely to > > > > corrupt the ring indexes, leaving the console unusable. > > > > > > > > Introduce a lock to xencons_info to serialize accesses to the shared > > > > ring. This is only required when using the shared memory console, > > > > concurrent accesses to the hypercall based console implementation are > > > > not an issue. > > > > > > > > Note the conditional logic in domU_read_console() is slightly modified > > > > so the notify_daemon() call can be done outside of the locked region: > > > > it's an hypercall and there's no need for it to be done with the lock > > > > held. > > > > > > For domU_read_console: I don't mean to block this patch but we need to > > > be sure about the semantics of hv_ops.get_chars. Either it is expected > > > to be already locked, then we definitely shouldn't add another lock to > > > domU_read_console. Or it is not expected to be already locked, then we > > > should add the lock. > > > > > > My impression is that it is expected to be already locked, but I think > > > we need Greg or Jiri to confirm one way or the other. > > > > Let me move both to the 'To:' field then. > > > > My main concern is the usage of hv_ops.get_chars hook in > > hvc_poll_get_char(), as it's not obvious to me that callers of > > tty->poll_get_char hook as returned by tty_find_polling_driver() will > > always do so with the tty lock held (in fact the only user right now > > doesn't seem to hold the tty lock). > > > > > Aside from that the rest looks fine. I guess I could reluctantly remove the lock in the get_chars hook, albeit I'm not convinced at all the lock is not needed there. Roger.
Re: [PATCH v1 2/2] arch/arm: time: Add support for parsing interrupts by names
Hi Michal,
> On 9 Mar 2023, at 11:05, Michal Orzel wrote:
>
>
>
> On 09/03/2023 09:02, Bertrand Marquis wrote:
>>
>>
>> Hi Stefano,
>>
>>> On 7 Mar 2023, at 22:02, Stefano Stabellini wrote:
>>>
>>> On Tue, 7 Mar 2023, Bertrand Marquis wrote:
> On 7 Mar 2023, at 11:09, Andrei Cherechesu (OSS)
> wrote:
>
> From: Andrei Cherechesu
>
> Added support for parsing the ARM generic timer interrupts DT
> node by the "interrupt-names" property, if it is available.
>
> If not available, the usual parsing based on the expected
> IRQ order is performed.
>
> Also added the "hyp-virt" PPI to the timer PPI list, even
> though it's currently not in use. If the "hyp-virt" PPI is
> not found, the hypervisor won't panic.
>
> Signed-off-by: Andrei Cherechesu
> ---
> xen/arch/arm/include/asm/time.h | 3 ++-
> xen/arch/arm/time.c | 26 ++
> 2 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/time.h
> b/xen/arch/arm/include/asm/time.h
> index 4b401c1110..49ad8c1a6d 100644
> --- a/xen/arch/arm/include/asm/time.h
> +++ b/xen/arch/arm/include/asm/time.h
> @@ -82,7 +82,8 @@ enum timer_ppi
> TIMER_PHYS_NONSECURE_PPI = 1,
> TIMER_VIRT_PPI = 2,
> TIMER_HYP_PPI = 3,
> -MAX_TIMER_PPI = 4,
> +TIMER_HYP_VIRT_PPI = 4,
> +MAX_TIMER_PPI = 5,
> };
>
> /*
> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
> index 433d7be909..794da646d6 100644
> --- a/xen/arch/arm/time.c
> +++ b/xen/arch/arm/time.c
> @@ -38,6 +38,14 @@ uint32_t __read_mostly timer_dt_clock_frequency;
>
> static unsigned int timer_irq[MAX_TIMER_PPI];
>
> +static const char *timer_irq_names[MAX_TIMER_PPI] = {
> +[TIMER_PHYS_SECURE_PPI] = "sec-phys",
> +[TIMER_PHYS_NONSECURE_PPI] = "phys",
> +[TIMER_VIRT_PPI] = "virt",
> +[TIMER_HYP_PPI] = "hyp-phys",
> +[TIMER_HYP_VIRT_PPI] = "hyp-virt",
> +};
> +
I would need some reference or a pointer to some doc to check those.
> unsigned int timer_get_irq(enum timer_ppi ppi)
> {
> ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI);
> @@ -149,15 +157,25 @@ static void __init init_dt_xen_time(void)
> {
> int res;
> unsigned int i;
> +bool has_names;
> +
> +has_names = dt_property_read_bool(timer, "interrupt-names");
>
> /* Retrieve all IRQs for the timer */
> for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
> {
> -res = platform_get_irq(timer, i);
> -
> -if ( res < 0 )
> +if ( has_names )
> +res = platform_get_irq_byname(timer, timer_irq_names[i]);
> +else
> +res = platform_get_irq(timer, i);
> +
> +if ( res > 0 )
The behaviour of the code is changed here compared to the current
version as res = 0 will now generate a panic.
Some device tree might not specify an interrupt number and just put
0 and Xen will now panic on those systems.
As I have no idea if such systems exists and the behaviour is modified
you should justify this and mention it in the commit message or keep
the old behaviour and let 0 go through without a panic.
@stefano, julien any idea here ? should just keep the old behaviour ?
>>>
>>> platform_get_irq returns 0 if the irq is 0. The irq cannot be 0 because
>>> 0 is reserved for SGIs, not PPIs. So I think it is OK to consider 0 an
>>> error.
>>
>> Problem here is that a DTB might not specify all interrupts and just put
>> 0 for the one not used (or not available for example if you have no secure
>> world).
> Xen requires presence of EL3,EL2 and on such system, at least the following
> timers needs to be there
> according to Arm ARM:
> - EL3 phys (if EL3 is there)
This might be needed by EL3 but not by Xen.
> - EL1 phys (always)
> - EL1 virt (always)
> - NS EL2 phys (if EL2 is there)
Agree
>
> therefore, if we get 0 for one of those, it means that something went wrong
> and we shall consider
> it as an error.
Agree except for the EL3 one :-)
Cheers
Bertrand
>
> ~Michal
Re: API/ABIs: Re: [RFC PATCH v2 0/2] Add a new acquire resource to query vcpu statistics
On Wed, Mar 08, 2023 at 03:16:05PM +0100, Jan Beulich wrote:
> On 08.03.2023 12:54, Matias Ezequiel Vara Larsen wrote:
> > On Tue, Mar 07, 2023 at 11:12:00AM +0100, Jan Beulich wrote:
> >> On 06.03.2023 15:23, Matias Ezequiel Vara Larsen wrote:
> >>> - Xen shall use the "stats_active" field to indicate what it is
> >>> producing. In
> >>> this field, reserved bits shall be 0. This shall allow us to agree on
> >>> the
> >>> layout even when producer and consumer are compiled with different
> >>> headers.
> >>
> >> I wonder how well such a bitfield is going to scale. It provides for
> >> only 32 (maybe 64) counters. Of course this may seem a lot right now,
> >> but you never know how quickly something like this can grow. Plus
> >> with ...
> >>
> >
> > Would it make sense to define it like this?:
> >
> > struct vcpu_shmem_stats {
> > #define STATS_A (1u << 0)
> > ...
> > #define VCPU_STATS_MAGIC 0xaabbccdd
> > uint32_t magic;
> > uint32_t offset; // roundup(sizeof(vcpu_shmem_stats) +
> > sizeof(uint32_t) * nr_stats, cacheline_size)
> > uint32_t size;// sizeof(vcpu_stats)
> > uint32_t stride; // roundup(sizeof(vcpu_stats), cacheline_size)
> > uint32_t nr_stats; // size of stats_active in uint32_t
> > uint32_t stats_active[XEN_FLEX_ARRAY_DIM];
> > ...
> > };
>
The use of stats_active[] is meant to have a bitmap that could scale thus not
limiting the number of counters in the vcpu_stat structure to 32 or 64. I can't
see other way to have an unlimited number of counters though.
> Possibly, but this would make it harder to use the interface. An alternative
> might be to specify that an actual stats value set to ~0 marks an inactive
> element. Since these are 64-bit counters, with today's and tomorrow's
> computers we won't be at risk of a counter reaching a value of 2^^64-1, I
> guess. And even if there was one where such a risk could not be excluded
> (e.g. because of using pretty large increments), one would merely need to
> make sure to saturate at 2^^64-2. Plus at such a point one would need to
> consider anyway to switch to 128-bit fields, as neither saturated nor
> wrapped values are of much use to consumers.
>
If I understand well, this use-case is in case an element in the stats_active
bitmap becomes inactive, i.e., it is set to "0" in stats_active[]. You are
proposing to set to ~0 the actual stats value to mark an inactive element. I
may be missing something here but would not be enough to set to "0" the
corresponding stats_active[] bit?
> >>> - In the vcpu_stats structure, new fields can only ever be appended.
> >>
> >> ... this rule the only ambiguity that could arise to consumers when
> >> no active flags existed would be that they can't tell "event never
> >> occurred" from "hypervisor doesn't know about this anymore".
> >
> > Do you mean how the consumer can figure out if either 1) Xen does not know
> > yet
> > about some flag or 2) the flag has been deprecated? I think 2) is the case
> > that
> > Andrew mention in which the magic number could be used as an ABI version to
> > break backwards compatibility.
>
> No, an inactive field wouldn't break the ABI. An ABI change would be if
> such an inactive field was actually removed from the array. Or if e.g.,
> as per above, we needed to switch to 128-bit counters.
Got it, Thanks.
Matias
Re: [PATCH v1 2/2] arch/arm: time: Add support for parsing interrupts by names
On 09/03/2023 09:02, Bertrand Marquis wrote:
>
>
> Hi Stefano,
>
>> On 7 Mar 2023, at 22:02, Stefano Stabellini wrote:
>>
>> On Tue, 7 Mar 2023, Bertrand Marquis wrote:
On 7 Mar 2023, at 11:09, Andrei Cherechesu (OSS)
wrote:
From: Andrei Cherechesu
Added support for parsing the ARM generic timer interrupts DT
node by the "interrupt-names" property, if it is available.
If not available, the usual parsing based on the expected
IRQ order is performed.
Also added the "hyp-virt" PPI to the timer PPI list, even
though it's currently not in use. If the "hyp-virt" PPI is
not found, the hypervisor won't panic.
Signed-off-by: Andrei Cherechesu
---
xen/arch/arm/include/asm/time.h | 3 ++-
xen/arch/arm/time.c | 26 ++
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/xen/arch/arm/include/asm/time.h
b/xen/arch/arm/include/asm/time.h
index 4b401c1110..49ad8c1a6d 100644
--- a/xen/arch/arm/include/asm/time.h
+++ b/xen/arch/arm/include/asm/time.h
@@ -82,7 +82,8 @@ enum timer_ppi
TIMER_PHYS_NONSECURE_PPI = 1,
TIMER_VIRT_PPI = 2,
TIMER_HYP_PPI = 3,
-MAX_TIMER_PPI = 4,
+TIMER_HYP_VIRT_PPI = 4,
+MAX_TIMER_PPI = 5,
};
/*
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 433d7be909..794da646d6 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -38,6 +38,14 @@ uint32_t __read_mostly timer_dt_clock_frequency;
static unsigned int timer_irq[MAX_TIMER_PPI];
+static const char *timer_irq_names[MAX_TIMER_PPI] = {
+[TIMER_PHYS_SECURE_PPI] = "sec-phys",
+[TIMER_PHYS_NONSECURE_PPI] = "phys",
+[TIMER_VIRT_PPI] = "virt",
+[TIMER_HYP_PPI] = "hyp-phys",
+[TIMER_HYP_VIRT_PPI] = "hyp-virt",
+};
+
>>>
>>> I would need some reference or a pointer to some doc to check those.
>>>
unsigned int timer_get_irq(enum timer_ppi ppi)
{
ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI);
@@ -149,15 +157,25 @@ static void __init init_dt_xen_time(void)
{
int res;
unsigned int i;
+bool has_names;
+
+has_names = dt_property_read_bool(timer, "interrupt-names");
/* Retrieve all IRQs for the timer */
for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
{
-res = platform_get_irq(timer, i);
-
-if ( res < 0 )
+if ( has_names )
+res = platform_get_irq_byname(timer, timer_irq_names[i]);
+else
+res = platform_get_irq(timer, i);
+
+if ( res > 0 )
>>>
>>> The behaviour of the code is changed here compared to the current
>>> version as res = 0 will now generate a panic.
>>>
>>> Some device tree might not specify an interrupt number and just put
>>> 0 and Xen will now panic on those systems.
>>> As I have no idea if such systems exists and the behaviour is modified
>>> you should justify this and mention it in the commit message or keep
>>> the old behaviour and let 0 go through without a panic.
>>>
>>> @stefano, julien any idea here ? should just keep the old behaviour ?
>>
>> platform_get_irq returns 0 if the irq is 0. The irq cannot be 0 because
>> 0 is reserved for SGIs, not PPIs. So I think it is OK to consider 0 an
>> error.
>
> Problem here is that a DTB might not specify all interrupts and just put
> 0 for the one not used (or not available for example if you have no secure
> world).
Xen requires presence of EL3,EL2 and on such system, at least the following
timers needs to be there
according to Arm ARM:
- EL3 phys (if EL3 is there)
- EL1 phys (always)
- EL1 virt (always)
- NS EL2 phys (if EL2 is there)
therefore, if we get 0 for one of those, it means that something went wrong and
we shall consider
it as an error.
~Michal
Re: [PATCH v6 2/4] xen: change to
On 08.03.2023 18:25, Oleksii wrote: > On Wed, 2023-03-08 at 16:27 +0100, Jan Beulich wrote: >> On 07.03.2023 16:50, Oleksii Kurochko wrote: >>> --- a/xen/arch/arm/include/asm/bug.h >>> +++ b/xen/arch/arm/include/asm/bug.h >>> @@ -1,6 +1,8 @@ >>> #ifndef __ARM_BUG_H__ >>> #define __ARM_BUG_H__ >>> >>> +#include >>> + >>> #if defined(CONFIG_ARM_32) >>> # include >>> #elif defined(CONFIG_ARM_64) >>> @@ -9,10 +11,16 @@ >>> # error "unknown ARM variant" >>> #endif >>> >>> +#undef BUG_DISP_WIDTH >>> +#undef BUG_LINE_LO_WIDTH >>> +#undef BUG_LINE_HI_WIDTH >> >> Why? For Arm ... >> >>> #define BUG_DISP_WIDTH 24 >>> #define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH) >>> #define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH) >> >> ... you could purge these as unused, even in a standalone prereq >> patch. >> And on x86 ... >> >>> --- a/xen/arch/x86/include/asm/bug.h >>> +++ b/xen/arch/x86/include/asm/bug.h >>> @@ -1,19 +1,18 @@ >>> #ifndef __X86_BUG_H__ >>> #define __X86_BUG_H__ >>> >>> +#undef BUG_DISP_WIDTH >>> +#undef BUG_LINE_LO_WIDTH >>> +#undef BUG_LINE_HI_WIDTH >>> + >>> #define BUG_DISP_WIDTH 24 >>> #define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH) >>> #define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH) >> >> ... there's no reason to #undef just to the #define again to the same >> values. All of this would be removed in a subsequent patch anyway, >> and >> it's less code churn (with code nevertheless being correct) if you >> get >> rid of the #define-s right away (as iirc you had it in an earlier >> version). If you agree then with these adjustments >> Reviewed-by: Jan Beulich > > It won't be compilation issue (redefinition) in the current one case > because defines are the same as in so it is possible to not > add #undef in this patch. Avoiding to add the #undef is the minimal approach. Yet as indicated I think the #define-s should also be dropped right here (x86) and in a prereq patch (Arm). Jan
Re: [PATCH v1 2/3] xen/riscv: setup initial pagetables
On 08.03.2023 17:16, Oleksii wrote:
> On Wed, 2023-03-08 at 16:17 +0100, Jan Beulich wrote:
>> On 08.03.2023 15:54, Oleksii wrote:
>>> Actually after my latest experiments it looks that we don't need to
>>> calculate that things at all because for RISC-V it is used
>>> everywhere
>>> PC-relative access.
>>> Thereby it doesn't matter what is an address where Xen was loaded
>>> and
>>> linker address.
>>> Right now I found only to cases which aren't PC-relative.
>>> Please look at the patch below:
>>> diff --git a/xen/arch/riscv/include/asm/config.h
>>> b/xen/arch/riscv/include/asm/config.h
>>> index 763a922a04..e1ba613d81 100644
>>> --- a/xen/arch/riscv/include/asm/config.h
>>> +++ b/xen/arch/riscv/include/asm/config.h
>>> @@ -39,7 +39,7 @@
>>> name:
>>> #endif
>>>
>>> -#define XEN_VIRT_START _AT(UL, 0x8020)
>>> +#define XEN_VIRT_START _AT(UL, 0x0020)
>>
>> I think this wants to remain the address where Xen actually runs, and
>> where Xen is linked to. This ...
>>
>>> --- a/xen/arch/riscv/traps.c
>>> +++ b/xen/arch/riscv/traps.c
>>> @@ -123,8 +123,14 @@ int do_bug_frame(const struct cpu_user_regs
>>> *regs,
>>> vaddr_t pc)
>>> const char *filename, *predicate;
>>> int lineno;
>>>
>>> - static const struct bug_frame* bug_frames[] = {
>>> - &__start_bug_frames[0],
>>> + /*
>>> + * force fill bug_frames array using auipc/addi instructions
>>> to
>>> + * make addresses in bug_frames PC-relative.
>>> + */
>>> + const struct bug_frame * force = (const struct bug_frame *)
>>> &__start_bug_frames[0];
>>> +
>>> + const struct bug_frame* bug_frames[] = {
>>> + force,
>>> &__stop_bug_frames_0[0],
>>> &__stop_bug_frames_1[0],
>>> &__stop_bug_frames_2[0],
>>
>> ... array would better be static anyway, and ...
>>
>>> The changes related to are only to make linker_addr
>>> !=
>>> load_address. So:
>>> 1. The first issue with cpu0_boot_stack in the head.S file. When we
>>> do:
>>> la sp, cpu0_boot_stack
>>> Pseudo-instruction la will be transformed to auipc/addi OR
>>> auipc/l{w|d}.
>>> It depends on an option: nopic, pic. [1]
>>>
>>> So the solution can be the following:
>>> * As it is done in the patch: add to the start of head.S
>>> ".option
>>> nopic"
>>> * Change la to lla thereby it will be always generated
>>> "auipc/addi"
>>> to get an address of variable.
>>>
>>> 2. The second issue is with the code in do_bug_frame() with
>>> bug_frames
>>> array:
>>> const struct bug_frame* bug_frames[] = {
>>> &__start_bug_frames[0],
>>> &__stop_bug_frames_0[0],
>>> &__stop_bug_frames_1[0],
>>> &__stop_bug_frames_2[0],
>>> &__stop_bug_frames_3[0],
>>> };
>>> In this case &{start,stop}bug_frames{,{0-3}} will be changed
>>> to
>>> linker address. In case of when load_addr is 0x8020 and
>>> linker_addr
>>> is 0x0020 then &{start,stop}bug_frames{,{0-3}} will be equal to
>>> 0x0020 + X.
>>
>> ... this "solution" to a problem you introduce by wrongly modifying
>> the linked address would then need applying to any other similar code
>> pattern found in Xen. Which is (I hope obviously) not a viable route.
>> Instead code running before address translation is enable needs to be
>> extra careful in what code and data items it accesses, and how.
>>
> I modified the linked address only for the experiment ( when load_addr
> != linker_addr to emulate situation Julien told me about), so it's not
> something I planned to send as a part of the final patch, and I
> probably forgot to mention that in my previous mail.
>
> It is only one place where we have to do a kind of 'force' and is
> needed to make the current state of RISC-V Xen work in case we don't
> have MMU enabled yet and linker_addr != load_addr. All other cases
> where it is used something from the range (linker_start, linker_end)
> will be managed by MMU.
>
> If we can't use mentioned above solution, we still need to handle the
> situation when linker_addr != load_addr and MMU isn't enabled yet.
> Other options to do that:
> 1. add phys_offset ( | load_addr - linker_addr | ) everywhere where
> bug_frames array is used: bug_frames[id] + phys_offset
Well, that again special cases a certain data structure. As said before,
you need to be very careful with any C code involved before translation
is enabled. Unless you want to retain relocations (so you can "move"
from load-time to link time addresses alongside enabling translation,
like we do on x86 in xen.efi), you want to constrain code paths as much
as possible. One approach is to move enabling of translation to early
assembly code (like we do on x86 for xen.gz). The other is to amend
involved code paths with something like what you say above.
> 2. Check somewhere at the start if linker_addr != load_addr, then throw
> an error and panic().
That's not really an option if the boot loader isn't required to place
the image at its linked address
Re: [RFC PATCH v2 1/2] xen/memory : Add a stats_table resource type
On Tue, Mar 07, 2023 at 05:55:26PM +0100, Jan Beulich wrote:
> On 07.03.2023 15:44, Matias Ezequiel Vara Larsen wrote:
> > On Thu, Feb 23, 2023 at 01:42:08PM +0100, Jan Beulich wrote:
> >> On 23.02.2023 13:16, Matias Ezequiel Vara Larsen wrote:
> >>> On Fri, Feb 17, 2023 at 03:10:53PM +0100, Jan Beulich wrote:
> On 17.02.2023 10:29, Matias Ezequiel Vara Larsen wrote:
> > On Fri, Feb 17, 2023 at 09:57:43AM +0100, Jan Beulich wrote:
> >> On 17.02.2023 09:50, Matias Ezequiel Vara Larsen wrote:
> >>> On Wed, Dec 14, 2022 at 08:56:57AM +0100, Jan Beulich wrote:
> On 14.12.2022 08:29, Jan Beulich wrote:
> > On 07.10.2022 14:39, Matias Ezequiel Vara Larsen wrote:
> >> +static int stats_vcpu_alloc_mfn(struct domain *d)
> >> +{
> >> +struct page_info *pg;
> >> +
> >> +pg = alloc_domheap_page(d, MEMF_no_refcount);
> >
> > The ioreq and vmtrace resources are also allocated this way, but
> > they're
> > HVM-specific. The one here being supposed to be VM-type
> > independent, I'm
> > afraid such pages will be accessible by an "owning" PV domain (it'll
> > need to guess the MFN, but that's no excuse).
>
> Which might be tolerable if it then can't write to the page. That
> would
> require "locking" the page r/o (from guest pov), which ought to be
> possible by leveraging a variant of what share_xen_page_with_guest()
> does: It marks pages PGT_none with a single type ref. This would mean
> ...
>
> >> +if ( !pg )
> >> +return -ENOMEM;
> >> +
> >> +if ( !get_page_and_type(pg, d, PGT_writable_page) ) {
>
> ... using PGT_none here. Afaict this _should_ work, but we have no
> precedent of doing so in the tree, and I may be overlooking something
> which prevents that from working.
>
> >>>
> >>> I do not fully understand this. I checked share_xen_page_with_guest()
> >>> and I
> >>> think you're talking about doing something like this for each
> >>> allocated page to
> >>> set them ro from a pv guest pov:
> >>>
> >>> pg->u.inuse.type_info = PGT_none;
> >>> pg->u.inuse.type_info |= PGT_validated | 1;
> >>> page_set_owner(page, d); // not sure if this is needed
> >>>
> >>> Then, I should use PGT_none instead of PGT_writable_page in
> >>> get_page_and_type(). Am I right?
> >>
> >> No, if at all possible you should avoid open-coding anything. As said,
> >> simply passing PGT_none to get_page_and_type() ought to work (again, as
> >> said, unless I'm overlooking something). share_xen_page_with_guest()
> >> can do what it does because the page isn't owned yet. For a page with
> >> owner you may not fiddle with type_info in such an open-coded manner.
> >>
> >
> > Thanks. I got the following bug when passing PGT_none:
> >
> > (XEN) Bad type in validate_page 0 t=0001 c=8042
> > (XEN) Xen BUG at mm.c:2643
>
> The caller of the function needs to avoid the call not only for writable
> and shared pages, but also for this new case of PGT_none.
> >>>
> >>> Thanks. If I understand correctly, _get_page_type() needs to avoid to call
> >>> validate_page() when type = PGT_none.
> >>
> >> Yes.
> >>
> >>> For the writable and shared pages, this
> >>> is avoided by setting nx |= PGT_validated. Am I right?
> >>
> >> Well, no, I wouldn't describe it like that. The two (soon three) types not
> >> requiring validation simply set the flag without calling validate_page().
> >>
> >
> > I see, thanks. I added the corresponding check at _get_page_type() to set
> > the
> > flag without calling validate_page() for the PGT_none type. I think I am
> > missing something when I am releasing the pages. I am triggering the
> > following
> > BUG() when issuing put_page_and_type():
> >
> > (XEN) Xen BUG at mm.c:2698
> >
> > This is at devalidate_page(). I guess the call to devalidate_page() shall be
> > also avoided.
>
> Well, yes, symmetry requires a change there as well. Here it's indirect:
> You want to avoid the call to _put_final_page_type(). That's enclosed by
> (nx & PGT_type_mask) <= PGT_l4_page_table, which happens to be true for
> PGT_none as well. There may be more instances of such a comparison, so
> it'll be necessary to find them and check whether they may now also be
> reached with PGT_none (looks like a comparison against PGT_root_page_table
> in _get_page_type() is also affected, albeit in a largely benign way).
>
Thanks. I could not find any other instance of that comparison except those
that you mention. I'll add a new patch in the series to deal with the
support for PGT_none.
Matias
Re: [linux-linus test] 179511: regressions - trouble: fail/pass/starved
On 09.03.2023 07:58, osstest service owner wrote:
> flight 179511 linux-linus real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/179511/
>
> Regressions :-(
>
> Tests which did not succeed and are blocking,
> including tests which could not be run:
> test-amd64-amd64-pair25 guest-start/debian fail REGR. vs.
> 178042
> test-amd64-amd64-freebsd12-amd64 16 guest-saverestorefail REGR. vs.
> 178042
> test-amd64-amd64-xl-credit1 18 guest-localmigrate fail REGR. vs.
> 178042
> test-amd64-amd64-xl-multivcpu 14 guest-start fail REGR. vs.
> 178042
> test-amd64-amd64-xl-pvshim 18 guest-localmigrate fail REGR. vs.
> 178042
> test-amd64-amd64-xl-shadow 18 guest-localmigrate fail REGR. vs.
> 178042
> test-amd64-amd64-xl 14 guest-start fail REGR. vs.
> 178042
While the xen-boot failure looks to be resolved now, there's at least
one further regression: Many instances of
(XEN) common/grant_table.c:2982:d0v3 copy beyond page area
can now be seen, which I expect are related to the test failure here.
Recent netback changes look benign, so it could be the sole recent
netfront one ("drivers: net: turn on XDP features") or something
elsewhere in the kernel (but presumably still in networking code).
The failure (and log messages) also ...
> test-amd64-amd64-freebsd11-amd64 13 guest-start fail REGR. vs. 178042
... occurring for freebsd suggests against the netfront change, though.
In any event ...
> test-arm64-arm64-xl 14 guest-start fail REGR. vs.
> 178042
... the issue looks to be arch-independent.
Jan
Re: [RFC PATCH 08/10] xen: pci: remove pcidev_[un]lock[ed] calls
On 09.03.2023 02:22, Volodymyr Babchuk wrote: > Jan Beulich writes: >> On 21.02.2023 00:13, Volodymyr Babchuk wrote: >>> Stefano Stabellini writes: On Wed, 31 Aug 2022, Volodymyr Babchuk wrote: > As pci devices are refcounted now and all list that store them are > protected by separate locks, we can safely drop global pcidevs_lock. > > Signed-off-by: Volodymyr Babchuk Up until this patch this patch series introduces: - d->pdevs_lock to protect d->pdev_list - pci_seg->alldevs_lock to protect pci_seg->alldevs_list - iommu->ats_list_lock to protect iommu->ats_devices - pdev refcounting to detect a pdev in-use and when to free it - pdev->lock to protect pdev->msi_list They cover a lot of ground. Are they collectively covering everything pcidevs_lock() was protecting? >>> >>> Well, that is the question. Those patch are in RFC stage because I can't >>> fully answer your question. I tried my best to introduce proper locking, >>> but apparently missed couple of places, like >>> deassign_device is not protected by pcidevs_lock anymore. deassign_device accesses a number of pdev fields, including quarantine, phantom_stride and fault.count. deassign_device could run at the same time as assign_device who sets quarantine and other fields. >>> >>> I hope this is all, but problem is that PCI subsystem is old, large and >>> complex. Fo example, as I wrote earlier, there are places that are >>> protected with pcidevs_lock(), but do nothing with PCI. I just don't >>> know what to do with such places. I have a hope that x86 maintainers >>> would review my changes and give feedback on missed spots. >> >> At the risk of it sounding unfair, at least initially: While review may >> spot issues, you will want to keep in mind that none of the people who >> originally wrote that code are around anymore. And even if they were, >> it would be uncertain - just like for the x86 maintainers - that they >> would recall (if they were aware at some time in the first place) all >> the corner cases. Therefore I'm afraid that proving correctness and >> safety of the proposed transformations can only be done by properly >> auditing all involved code paths. Yet that's something that imo wants >> to already have been done by the time patches are submitted for review. >> Reviewers would then "merely" (hard enough perhaps) check the results >> of that audit. >> >> I might guess that this locking situation is one of the reasons why >> Andrew in particular thinks (afaik) that the IOMMU code we have would >> better be re-written almost from scratch. I assume it's clear to him >> (it certainly is to me) that this is something that could only be >> expected to happen in an ideal work: I see no-one taking on such an >> exercise. We already have too little bandwidth. > > The more I dig into IOMMU code, the more I agree with Andrew. I can't > see how current PCI locking can be untangled in the IOMMU code. There > are just too many moving parts. I tried to play with static code > analysis tools, but I haven't find anything that can reliably analyze > locking in Xen. I even tried to write own tool tailored specifically for > PCI locking analysis. While it works on some synthetic tests, there is > too much work to support actual Xen code. > > I am not able to rework x86 IOMMU code. So, I am inclined to drop this > patch series at all. My current plan is to take minimal refcounting from > this series to satisfy your comments for "vpci: use pcidevs locking to > protect MMIO handlers". I guess this may indeed be the "best" approach for now - introduce refcounting to use where relevant for new work, and then slowly see about replacing (dropping) locking where a refcount suffices when one is held. Jan
Re: [BUG] x2apic broken with current AMD hardware
On 09.03.2023 00:08, Elliott Mitchell wrote: > On Wed, Mar 08, 2023 at 04:50:51PM +0100, Jan Beulich wrote: >> On 08.03.2023 16:23, Elliott Mitchell wrote: >>> Mostly SSIA. As originally identified by "Neowutran", appears Xen's >>> x2apic wrapper implementation fails with current generation AMD hardware >>> (Ryzen 7xxx/Zen 4). This can be worked around by passing "x2apic=false" >>> on Xen's command-line, though I'm wondering about the performance impact. >>> >>> There hasn't been much activity on xen-devel WRT x2apic, so a patch which >>> fixed this may have flown under the radar. Most testing has also been >>> somewhat removed from HEAD. >>> >>> Thanks to "Neowutran" for falling on this grenade and making it easier >>> for the followers. Pointer to first report: >>> https://forum.qubes-os.org/t/ryzen-7000-serie/14538 >> >> I'm sorry, but when you point at this long a report, would you please be a >> little more specific as to where the problem in question is actually >> mentioned? Searching the page for "x2apic" didn't give any hits at all >> until I first scrolled to the bottom of the (at present) 95 comments. And >> then while there are five mentions, there's nothing I could spot that >> would actually help understanding what is actually wrong. A statement like >> "... is because the implementation of x2apic is incorrect" isn't helpful >> on its own. And a later statement by another person puts under question >> whether "x2apic=false" actually helps in all cases. >> >> Please can we get a proper bug report here with suitable technical detail? >> Or alternatively a patch to discuss? > > Mostly I was pointing to the thread to credit Neowutran and company with > originally finding the workaround. I'm concerned about how > representative my reproduction is since the computer in question is > presently using Debian's build of Xen, 4.14. > > As such I'm less than certain the problem is still in HEAD, though > Neowutran and Co working with 4.16 and the commit log being quiet > suggests there is a good chance. > > More detail, pretty well most things are broken for Domain 0 without > "x2apic=false". Trying to boot with a 6.1.12 a USB keyboard was > completely unresponsive, on screen the initial ramdisk script output was > indicating problems interacting with storage devices. Those two together > suggested an interrupt issue and adding "x2apic=false" caused domain 0 to > successfully boot. > A 5.10 kernel similarly requires "x2apic=false" to successfully boot. > > So could be a commit after 4.16 fixed x2apic for current AMD hardware, > but may still be broken. If Dom0 boot is affected, trying a newer hypervisor shouldn't be a problem. You won't need any of the toolstack to match just to see whether Dom0 boots. In any event you will want to collect a serial log at maximum verbosity. It would also be of interest to know whether turning off the IOMMU avoids the issue as well (on the assumption that your system has less than 255 CPUs). Jan
Re: [PATCH V2] docs: vhost-user: Add Xen specific memory mapping support
On 07-03-23, 11:22, Stefan Hajnoczi wrote: > VHOST_USER_IOTLB_MSG probably isn't necessary because address > translation is not required. It will also reduce performance by adding > extra communication. > > Instead, you could change the 1 memory region : 1 mmap relationship that > existing non-Xen vhost-user back-end implementations have. In Xen > vhost-user back-ends, the memory region details (including the file > descriptor and Xen domain id) would be stashed away in back-end when the > front-end adds memory regions. No mmap would be performed upon > VHOST_USER_ADD_MEM_REG or VHOST_USER_SET_MEM_TABLE. > > Whenever the back-end needs to do DMA, it looks up the memory region and > performs the mmap + Xen-specific calls: > - A long-lived mmap of the vring is set up when > VHOST_USER_SET_VRING_ENABLE is received. > - Short-lived mmaps of the indirect descriptors and memory pointed to by > the descriptors is set up by the virtqueue processing code. > > Does this sound workable to you? Sounds good. I have sent a proposal (v3) based on that now. -- viresh
[PATCH V3 1/2] docs: vhost-user: Define memory region separately
The same layout is defined twice, once in "single memory region description" and then in "memory regions description". Separate out details of memory region from these two and reuse the same definition later on. While at it, also rename "memory regions description" to "multiple memory regions description", to avoid potential confusion around similar names. And define single region before multiple ones. This is just a documentation optimization, the protocol remains the same. Signed-off-by: Viresh Kumar --- docs/interop/vhost-user.rst | 39 + 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst index 8a5924ea75ab..1720d681264d 100644 --- a/docs/interop/vhost-user.rst +++ b/docs/interop/vhost-user.rst @@ -130,18 +130,8 @@ A vring address description Note that a ring address is an IOVA if ``VIRTIO_F_IOMMU_PLATFORM`` has been negotiated. Otherwise it is a user address. -Memory regions description -^^ - -+-+-+-+-+-+ -| num regions | padding | region0 | ... | region7 | -+-+-+-+-+-+ - -:num regions: a 32-bit number of regions - -:padding: 32-bit - -A region is: +Memory region description +^ +---+--+--+-+ | guest address | size | user address | mmap offset | @@ -158,19 +148,26 @@ Memory regions description Single memory region description -+-+---+--+--+-+ -| padding | guest address | size | user address | mmap offset | -+-+---+--+--+-+ ++-++ +| padding | region | ++-++ :padding: 64-bit -:guest address: a 64-bit guest address of the region +A region is represented by Memory region description. -:size: a 64-bit size +Multiple Memory regions description +^^^ -:user address: a 64-bit user address ++-+-+-+-+-+ +| num regions | padding | region0 | ... | region7 | ++-+-+-+-+-+ -:mmap offset: 64-bit offset where region starts in the mapped memory +:num regions: a 32-bit number of regions + +:padding: 32-bit + +A region is represented by Memory region description. Log description ^^^ @@ -952,8 +949,8 @@ Front-end message types ``VHOST_USER_SET_MEM_TABLE`` :id: 5 :equivalent ioctl: ``VHOST_SET_MEM_TABLE`` - :request payload: memory regions description - :reply payload: (postcopy only) memory regions description + :request payload: multiple memory regions description + :reply payload: (postcopy only) multiple memory regions description Sets the memory map regions on the back-end so it can translate the vring addresses. In the ancillary data there is an array of file -- 2.31.1.272.g89b43f80a514
[PATCH V3 0/2] qemu: vhost-user: Support Xen memory mapping quirks
Hello, This patchset tries to update the vhost-user protocol to make it support special memory mapping required in case of Xen hypervisor. The first patch is mostly cleanup and second one introduces a new xen specific feature. V2->V3: - Remove the extra message and instead update the memory regions to carry additional data. - Drop the one region one mmap relationship and allow back-end to map only parts of a region at once, required for Xen grant mappings. - Additional cleanup patch 1/2. V1->V2: - Make the custom mmap feature Xen specific, instead of being generic. - Clearly define which memory regions are impacted by this change. - Allow VHOST_USER_SET_XEN_MMAP to be called multiple times. - Additional Bit(2) property in flags. Viresh Kumar (2): docs: vhost-user: Define memory region separately docs: vhost-user: Add Xen specific memory mapping support docs/interop/vhost-user.rst | 60 - 1 file changed, 39 insertions(+), 21 deletions(-) -- 2.31.1.272.g89b43f80a514
[PATCH V3 2/2] docs: vhost-user: Add Xen specific memory mapping support
The current model of memory mapping at the back-end works fine where a standard call to mmap() (for the respective file descriptor) is enough before the front-end can start accessing the guest memory. There are other complex cases though where the back-end needs more information and simple mmap() isn't enough. For example Xen, a type-1 hypervisor, currently supports memory mapping via two different methods, foreign-mapping (via /dev/privcmd) and grant-dev (via /dev/gntdev). In both these cases, the back-end needs to call mmap() and ioctl(), with extra information like the Xen domain-id of the guest whose memory we are trying to map. Add a new protocol feature, 'VHOST_USER_PROTOCOL_F_XEN_MMAP', which lets the back-end know about the additional memory mapping requirements. When this feature is negotiated, the front-end will send the additional information within the memory regions themselves. Signed-off-by: Viresh Kumar --- docs/interop/vhost-user.rst | 21 + 1 file changed, 21 insertions(+) diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst index 1720d681264d..5a070adbc1aa 100644 --- a/docs/interop/vhost-user.rst +++ b/docs/interop/vhost-user.rst @@ -145,6 +145,26 @@ Memory region description :mmap offset: 64-bit offset where region starts in the mapped memory +When the ``VHOST_USER_PROTOCOL_F_XEN_MMAP`` protocol feature has been +successfully negotiated, the memory region description contains two extra +fields at the end. + ++---+--+--+-++---+ +| guest address | size | user address | mmap offset | xen mmap flags | domid | ++---+--+--+-++---+ + +:xen mmap flags: 32-bit bit field + +- Bit 0 is set for Xen foreign memory mapping. +- Bit 1 is set for Xen grant memory mapping. +- Bit 8 is set if the memory region can not be mapped in advance, and memory + areas within this region must be mapped / unmapped only when required by the + back-end. The back-end shouldn't try to map the entire region at once, as the + front-end may not allow it. The back-end should rather map only the required + amount of memory at once and unmap it after it is used. + +:domid: a 32-bit Xen hypervisor specific domain id. + Single memory region description @@ -864,6 +884,7 @@ Protocol features #define VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS 14 #define VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS 15 #define VHOST_USER_PROTOCOL_F_STATUS 16 + #define VHOST_USER_PROTOCOL_F_XEN_MMAP 17 Front-end message types --- -- 2.31.1.272.g89b43f80a514
Re: [PATCH v1 2/2] arch/arm: time: Add support for parsing interrupts by names
Hi Stefano,
> On 7 Mar 2023, at 22:02, Stefano Stabellini wrote:
>
> On Tue, 7 Mar 2023, Bertrand Marquis wrote:
>>> On 7 Mar 2023, at 11:09, Andrei Cherechesu (OSS)
>>> wrote:
>>>
>>> From: Andrei Cherechesu
>>>
>>> Added support for parsing the ARM generic timer interrupts DT
>>> node by the "interrupt-names" property, if it is available.
>>>
>>> If not available, the usual parsing based on the expected
>>> IRQ order is performed.
>>>
>>> Also added the "hyp-virt" PPI to the timer PPI list, even
>>> though it's currently not in use. If the "hyp-virt" PPI is
>>> not found, the hypervisor won't panic.
>>>
>>> Signed-off-by: Andrei Cherechesu
>>> ---
>>> xen/arch/arm/include/asm/time.h | 3 ++-
>>> xen/arch/arm/time.c | 26 ++
>>> 2 files changed, 24 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/include/asm/time.h
>>> b/xen/arch/arm/include/asm/time.h
>>> index 4b401c1110..49ad8c1a6d 100644
>>> --- a/xen/arch/arm/include/asm/time.h
>>> +++ b/xen/arch/arm/include/asm/time.h
>>> @@ -82,7 +82,8 @@ enum timer_ppi
>>>TIMER_PHYS_NONSECURE_PPI = 1,
>>>TIMER_VIRT_PPI = 2,
>>>TIMER_HYP_PPI = 3,
>>> -MAX_TIMER_PPI = 4,
>>> +TIMER_HYP_VIRT_PPI = 4,
>>> +MAX_TIMER_PPI = 5,
>>> };
>>>
>>> /*
>>> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
>>> index 433d7be909..794da646d6 100644
>>> --- a/xen/arch/arm/time.c
>>> +++ b/xen/arch/arm/time.c
>>> @@ -38,6 +38,14 @@ uint32_t __read_mostly timer_dt_clock_frequency;
>>>
>>> static unsigned int timer_irq[MAX_TIMER_PPI];
>>>
>>> +static const char *timer_irq_names[MAX_TIMER_PPI] = {
>>> +[TIMER_PHYS_SECURE_PPI] = "sec-phys",
>>> +[TIMER_PHYS_NONSECURE_PPI] = "phys",
>>> +[TIMER_VIRT_PPI] = "virt",
>>> +[TIMER_HYP_PPI] = "hyp-phys",
>>> +[TIMER_HYP_VIRT_PPI] = "hyp-virt",
>>> +};
>>> +
>>
>> I would need some reference or a pointer to some doc to check those.
>>
>>> unsigned int timer_get_irq(enum timer_ppi ppi)
>>> {
>>>ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI);
>>> @@ -149,15 +157,25 @@ static void __init init_dt_xen_time(void)
>>> {
>>>int res;
>>>unsigned int i;
>>> +bool has_names;
>>> +
>>> +has_names = dt_property_read_bool(timer, "interrupt-names");
>>>
>>>/* Retrieve all IRQs for the timer */
>>>for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
>>>{
>>> -res = platform_get_irq(timer, i);
>>> -
>>> -if ( res < 0 )
>>> +if ( has_names )
>>> +res = platform_get_irq_byname(timer, timer_irq_names[i]);
>>> +else
>>> +res = platform_get_irq(timer, i);
>>> +
>>> +if ( res > 0 )
>>
>> The behaviour of the code is changed here compared to the current
>> version as res = 0 will now generate a panic.
>>
>> Some device tree might not specify an interrupt number and just put
>> 0 and Xen will now panic on those systems.
>> As I have no idea if such systems exists and the behaviour is modified
>> you should justify this and mention it in the commit message or keep
>> the old behaviour and let 0 go through without a panic.
>>
>> @stefano, julien any idea here ? should just keep the old behaviour ?
>
> platform_get_irq returns 0 if the irq is 0. The irq cannot be 0 because
> 0 is reserved for SGIs, not PPIs. So I think it is OK to consider 0 an
> error.
Problem here is that a DTB might not specify all interrupts and just put
0 for the one not used (or not available for example if you have no secure
world).
So I think we need to keep the current behaviour, might be ok to put a
debug print.
What I would think is feasible would be to panic for interrupt numbers we
need only.
Cheers
Bertrand
