RE: [PATCH v6 07/11] xen/arm: implement FIXMAP_ADDR for MPU systems
Hi Julien, > -Original Message- > From: Julien Grall > Sent: 2022年11月10日 2:30 > To: Wei Chen ; xen-devel@lists.xenproject.org > Cc: nd ; Stefano Stabellini ; Bertrand > Marquis ; Volodymyr Babchuk > > Subject: Re: [PATCH v6 07/11] xen/arm: implement FIXMAP_ADDR for MPU > systems > > > > On 09/11/2022 06:46, Wei Chen wrote: > > Hi Julien, > > Hi Wei, > > > > >> -Original Message- > >> From: Julien Grall > >> Sent: 2022年11月7日 3:45 > >> To: Wei Chen ; xen-devel@lists.xenproject.org > >> Cc: nd ; Stefano Stabellini ; > Bertrand > >> Marquis ; Volodymyr Babchuk > >> > >> Subject: Re: [PATCH v6 07/11] xen/arm: implement FIXMAP_ADDR for MPU > >> systems > >> > >> Hi Wei, > >> > >> On 04/11/2022 10:07, Wei Chen wrote: > >>> FIXMAP is a special virtual address section for Xen to map some > >>> physical ram or device memory temporarily in initialization for > >>> MMU systems. FIXMAP_ADDR will return a virtual address by index > >>> for special purpose phys-to-virt mapping usage. For example, > >>> FIXMAP_ADDR(FIXMAP_CONSOLE) for early console mapping and > >>> FIXMAP_ADDR(FIXMAP_MISC) for copy_from_paddr. > >> > >> To me, we are bending quite a bit the definition of the fixmap. There > >> are not many use of the FIXMAP within the code and I think it would > >> simply be better to abstract the use (or removing it when possible) and > >> avoid defining FIXMAP_ADDR() & co for MPU. > >> > > > > I agree, if we don't mind to add some CONFIG_HAS_MPU in some generic > code. > > FAOD, this is not what I had in mind. Instead, it was to provide helper > which for !HAS_MPU would call fixmap and for HAS_MPU would do the work > to map the region in the MPU. > Sorry, I am still confused about this comment, did you mean we can provider Some generic helpers like: early_map_console / eary_map_guest_memory. For non-MPU system, we still can call fixmap in these callers, but for MPU system, we have to map the region to MPU region? > [...] > > >>>xen/arch/arm/Kconfig | 2 +- > >>>xen/arch/arm/include/asm/config_mpu.h | 2 ++ > >>>xen/arch/arm/include/asm/fixmap.h | 25 > + > >>>3 files changed, 28 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig > >>> index ac276307d6..1458ffa777 100644 > >>> --- a/xen/arch/arm/Kconfig > >>> +++ b/xen/arch/arm/Kconfig > >>> @@ -16,7 +16,7 @@ config ARM > >>> select HAS_DEVICE_TREE > >>> select HAS_PASSTHROUGH > >>> select HAS_PDX > >>> - select HAS_PMAP > >>> + select HAS_PMAP if !HAS_MPU > >> > >> I can't find any change of mm.c in this series. So surely this will > >> break the build? > > > > Yes, in our internal testing, open PMAP for MPU will cause building > > failed, except we add some new stubs for MPU system. > > Do you mean you added some stubs for PMAP? If so, I would not expect any > caller for the pmap() to be used for the MPU. Therefore, why would they > be necessary? > No, I mean if we want to make pmap can be built successfully for MPU, we have to implement some stubs like: fix_to_virt, xen_fixmap and write_pte, to make compiling success. But just as you said, we would not expect MPU to use any PMAP function, so we have not implemented them for MPU. Instead we disable PMAP for MPU. Cheer, Wei Chen > Cheers, > > -- > Julien Grall
Re: [PATCH for-4.17 v2] hvm/apic: repurpose the reporting of the APIC assist options
On 10.11.2022 23:47, Andrew Cooper wrote: > On 04/11/2022 16:18, Roger Pau Monne wrote: >> --- a/xen/arch/x86/hvm/viridian/viridian.c >> +++ b/xen/arch/x86/hvm/viridian/viridian.c >> @@ -197,7 +197,7 @@ void cpuid_viridian_leaves(const struct vcpu *v, >> uint32_t leaf, >> res->a = CPUID4A_RELAX_TIMER_INT; >> if ( viridian_feature_mask(d) & HVMPV_hcall_remote_tlb_flush ) >> res->a |= CPUID4A_HCALL_REMOTE_TLB_FLUSH; >> -if ( !cpu_has_vmx_apic_reg_virt ) >> +if ( !has_assisted_xapic(d) ) >> res->a |= CPUID4A_MSR_BASED_APIC; > > This check is broken before and after. It needs to be keyed on > virtualised interrupt delivery, not register acceleration. To me this connection you suggest looks entirely unobvious, so would you mind expanding as to why you're thinking so? The hint to the guest here is related to how it would best access certain registers (aiui), which to me looks orthogonal to how interrupt delivery works. Jan
[ovmf test] 174730: all pass - PUSHED
flight 174730 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/174730/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf c8fb7240469b5753773da4fa5710b870b790c363 baseline version: ovmf 342813a3f7794bf67405a236053f27c916804d36 Last test of basis 174729 2022-11-11 01:10:36 Z0 days Testing same since 174730 2022-11-11 03:48:41 Z0 days1 attempts People who touched revisions under test: Ard Biesheuvel Michael D Kinney Sainadh Nagolu 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 342813a3f7..c8fb724046 c8fb7240469b5753773da4fa5710b870b790c363 -> xen-tested-master
[xen-unstable test] 174724: tolerable FAIL - PUSHED
flight 174724 xen-unstable real [real] flight 174732 xen-unstable real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/174724/ http://logs.test-lab.xenproject.org/osstest/logs/174732/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-pair11 xen-install/dst_host fail pass in 174732-retest Tests which did not succeed, but are not blocking: test-amd64-i386-freebsd10-amd64 7 xen-installfail like 174701 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 174701 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 174701 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 174701 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 174701 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 174701 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 174701 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 174701 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 174701 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 174701 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 174701 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 174701 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 174701 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 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-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail ne
RE: Xen Arm vpl011 UART will cause segmentation fault in Linux guest
Hi > -Original Message- > From: Stefano Stabellini > Sent: Friday, November 11, 2022 4:33 AM > To: Michal Orzel > Cc: Jiamei Xie ; xen-devel@lists.xenproject.org; Wei > Chen ; Bertrand Marquis > ; jul...@xen.org; sstabell...@kernel.org > Subject: Re: Xen Arm vpl011 UART will cause segmentation fault in Linux > guest > > On Wed, 9 Nov 2022, Michal Orzel wrote: > > Hi Jiamei, > > > > On 09/11/2022 09:25, Jiamei Xie wrote: > > > > > > > > > Hi Michal， > > > > > > Below log can be got when stating the linux guest. It says 9c09 is sbsa. > And 9c09 is also output > > > in bootlogd error message: > > > Serial: AMBA PL011 UART driver > > > 9c0b.uart: ttyAMA0 at MMIO 0x9c0b (irq = 12, base_baud = 0) > is a PL011 rev2 > > > printk: console [ttyAMA0] enabled > > > 9c09.sbsa-uart: ttyAMA1 at MMIO 0x9c09 (irq = 15, base_baud > = 0) is a SBSA > > > > > > > Xen behavior is correct and this would be Linux fault to try to write to > DMACR for SBSA UART device. > > DMACR is just an example. If you try to program e.g. the baudrate (through > LCR) for VPL011 it will > > also result in injecting abort into the guest. Should Xen support it? No. > > The > reason why is that > > it is not spec compliant operation. SBSA specification directly specifies > what registers are exposed. > > If Linux tries to write to some of the none-spec compliant registers - it > > is its > fault. > > Yeah, we need to fix Linux. > > FYI this is not the first bug in Linux affecting the sbsa-uart driver: > the issue is that the pl011 driver and the sbsa-uart driver share the > same code in Linux so it happens sometimes that a pl011-only feature > creeps into the sbsa-uart driver by mistake. Thanks for your confirm about this. In that case, I will check the Linux code to see why this happens and how to fix it. Best wishes Jiamei Xie > > > > >> -Original Message- > > >> From: Michal Orzel > > >> Sent: Wednesday, November 9, 2022 3:40 PM > > >> To: Jiamei Xie ; xen-devel@lists.xenproject.org > > >> Cc: Wei Chen ; Bertrand Marquis > > >> ; jul...@xen.org; sstabell...@kernel.org > > >> Subject: Re: Xen Arm vpl011 UART will cause segmentation fault in Linux > > >> guest > > >> > > >> Hi Jiamei, > > >> > > >> On 09/11/2022 08:20, Jiamei Xie wrote: > > >>> > > >>> > > >>> Hi all, > > >>> > > >>> When the guest kernel enables DMA engine with > > >> "CONFIG_DMA_ENGINE=y", Linux AMBA PL011 driver will access PL011 > > >> DMACR register. But this register have not been supported by vpl011 of > Xen. > > >> Xen will inject a data abort into guest, this will cause segmentation > > >> fault > of > > >> guest with the below message: > > >> I am quite confused. > > >> VPL011 implements SBSA UART which only implements some subset of > PL011 > > >> operations (SBSA UART is not PL011). > > >> According to spec (SBSA ver. 6.0), the SBSA_UART does not support > DMA > > >> features so Xen code is fine. > > >> When Xen exposes vpl011 device to a guest, this device has "arm,sbsa- > uart" > > >> compatible and not "uart-pl011". > > >> Linux driver "amba-pl011.c" should see this compatible and assign > proper > > >> operations (sbsa_uart_pops instead of amba_pl011_pops) that do not > enable > > >> DMA. > > >> Maybe the issue is with your configuration? > > >> > > >> ~Michal > > >> > > >>> Unhandled fault at 0xffc00944d048 > > >>> Mem abort info: > > >>> ESR = 0x9600 > > >>> EC = 0x25: DABT (current EL), IL = 32 bits > > >>> SET = 0, FnV = 0 > > >>> EA = 0, S1PTW = 0 > > >>> FSC = 0x00: ttbr address size fault > > >>> Data abort info: > > >>> ISV = 0, ISS = 0x > > >>> CM = 0, WnR = 0 > > >>> swapper pgtable: 4k pages, 39-bit VAs, pgdp=20e2e000 > > >>> [ffc00944d048] pgd=10003803, p4d=10003803, > > >> pud=10003803, pmd=10003fffa803, > pte=00689c090f13 > > >>> Internal error: ttbr address size fault: 9600 [#1] PREEMPT SMP > > >>> Modules linked in: > > >>> CPU: 0 PID: 132 Comm: bootlogd Not tainted 5.15.44-yocto-standard > #1 > > >>> pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > > >>> pc : pl011_stop_rx+0x70/0x80 > > >>> lr : uart_tty_port_shutdown+0x44/0x110 > > >>> sp : ffc00999bba0 > > >>> x29: ffc00999bba0 x28: ff80234ac380 x27: ff8022f5d000 > > >>> x26: x25: 45585401 x24: > > > >>> x23: ff8021ba4660 x22: 0001 x21: ff8021a0e2a0 > > >>> x20: ff802198f880 x19: ff8021a0e1a0 x18: > > >>> x17: x16: x15: > > > >>> x14: x13: x12: > > > >>> x11: x10: x9 : > ffc00871ba14 > > >>> x8 : ffc0099de260 x7 : ff8021a0e318 x6 : 0003 > > >>> x5 : ffc009315f20 x4 : ffc00944d038 x3 : > > >>> x2 : ffc00944d048 x1 : x0 : 0048 > > >>> Call trace: > > >>> pl0
[ovmf test] 174729: all pass - PUSHED
flight 174729 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/174729/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 342813a3f7794bf67405a236053f27c916804d36 baseline version: ovmf b0fd3097193d9c6825979e57e78e6278163bfd8e Last test of basis 174692 2022-11-09 15:13:33 Z1 days Testing same since 174729 2022-11-11 01:10:36 Z0 days1 attempts People who touched revisions under test: Michael D Kinney 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 b0fd309719..342813a3f7 342813a3f7794bf67405a236053f27c916804d36 -> xen-tested-master
[qemu-mainline test] 174708: tolerable FAIL - PUSHED
flight 174708 qemu-mainline real [real] flight 174728 qemu-mainline real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/174708/ http://logs.test-lab.xenproject.org/osstest/logs/174728/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-pair11 xen-install/dst_host fail pass in 174728-retest Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 174687 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 174687 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 174687 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 174687 test-amd64-amd64-xl-qcow221 guest-start/debian.repeatfail like 174687 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 174687 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 174687 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 174687 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 174687 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-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-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-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-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-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-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass version targeted for testing: qemuu2ccad61746ca7de5dd3e25146062264387e43bd4 baseline version: qemuu60ab36907ded291
Re: Porting Xen in raspberry pi4B
Hi Vipul, Sorry for the late reply. From the earlier logs that you sent, it looks like everything should be working correctly. Specifically: vfb = "" 1 = "" 0 = "" frontend = "/local/domain/1/device/vfb/0" frontend-id = "1" online = "1" state = "4" vnc = "1" vnclisten = "127.0.0.1" vncdisplay = "0" vncunused = "1" sdl = "0" opengl = "0" feature-resize = "1" hotplug-status = "connected" request-update = "1" state "4" means "connected". So I would expect that you should be able to connect to the vnc server using vncviewer. You might not see anything (black screen) but you should definitely be able to connect. I wouldn't try to launch x11 in the guest just yet. fbcon in Linux is enough to render something on the screen. You should be able to see the Linux text-based console rendered graphically, connecting to it via vnc. Sorry for the basic question, but have you tried all the following? vncviewer 127.0.0.1:0 vncviewer 127.0.0.1:1 vncviewer 127.0.0.1:2 vncviewer 127.0.0.1:5900 vncviewer 127.0.0.1:5901 vncviewer 127.0.0.1:5902 Given that from the xenstore-ls logs everything seems to work correctly I am not sure what else to suggest. You might have to add printf to QEMU ui/vnc.c and hw/display/xenfb.c to see what is going wrong. Cheers, Stefano On Mon, 7 Nov 2022, Vipul Suneja wrote: > Hi Stefano, > Thanks! > > Any input further on "xenstore-ls" logs? > > I am trying to run the x0vncserver & x11vnc server manually on guest > machine(xen_guest_image_minimal) image but it's failing with the below > error. > > root@raspberrypi4-64:/usr/bin# x0vncserver > x0vncserver: unable to open display "" > root@raspberrypi4-64:/usr/bin# > root@raspberrypi4-64:/usr/bin# x11vnc > ### > #@# > #@ @# > #@ ** WARNING ** WARNING ** WARNING ** WARNING ** @# > #@ @# > #@ YOU ARE RUNNING X11VNC WITHOUT A PASSWORD!! @# > #@ @# > #@ This means anyone with network access to this computer @# > #@ may be able to view and control your desktop. @# > #@ @# > #@ >>> If you did not mean to do this Press CTRL-C now!! <<< @# > #@ @# > #@# > #@ @# > #@ You can create an x11vnc password file by running: @# > #@ @# > #@ x11vnc -storepasswd password /path/to/passfile @# > #@ or x11vnc -storepasswd /path/to/passfile @# > #@ or x11vnc -storepasswd @# > #@ @# > #@ (the last one will use ~/.vnc/passwd) @# > #@ @# > #@ and then starting x11vnc via: @# > #@ @# > #@ x11vnc -rfbauth /path/to/passfile @# > #@ @# > #@ an existing ~/.vnc/passwd file from another VNC @# > #@ application will work fine too. @# > #@ @# > #@ You can also use the -passwdfile or -passwd options. @# > #@ (note -passwd is unsafe if local users are not trusted) @# > #@ @# > #@ Make sure any -rfbauth and -passwdfile password files @# > #@ cannot be read by untrusted users. @# > #@ @# > #@ Use x11vnc -usepw to automatically use your @# > #@ ~/.vnc/passwd or ~/.vnc/passwdfile password files. @# > #@ (and prompt you to create ~/.vnc/passwd if neither @# > #@ file exists.) Under -usepw, x11vnc will exit if it @# > #@ cannot find a password to use. @# > #@ @# > #@ @# > #@ Even with a password, the subsequent VNC traffic is @# > #@ sent in the clear. Consider tunnelling via ssh(1): @# > #@ @# > #@ http://www.karlrunge.com/x11vnc/#tunnelling @# > #@ @# > #@ Or using the x11vnc SSL options: -ssl and -stunnel @# > #@
[libvirt test] 174706: tolerable all pass - PUSHED
flight 174706 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/174706/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 174685 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 174685 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 174685 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-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt 16 saverestore-support-checkfail never pass test-amd64-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-amd64-amd64-libvirt-vhd 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-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass version targeted for testing: libvirt 2e16c9f20280fc051adea7544600fdc351415739 baseline version: libvirt 640e1050bf756c8ccec65cdd6e7ce544bae764db Last test of basis 174685 2022-11-09 04:22:09 Z1 days Testing same since 174706 2022-11-10 04:18:51 Z0 days1 attempts People who touched revisions under test: Göran Uddeborg Michal Privoznik Peter Krempa Roman Bogorodskiy Stefan Berger jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-libvirt-xsm pass test-arm64-arm64-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt pass test-armhf-armhf-libvirt pass test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair pass test-arm64-arm64-libvirt-qcow2 pass test-armhf-armhf-libvirt-qcow2 pass test-arm64-arm64-libvirt-raw pass test-armhf-armhf-libvirt-raw pass test-amd64-i386-libvirt-raw pass test-amd64-amd64-libvirt-vhd pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/oss
Re: [XEN PATCH for-4.17 v2 0/6] Fixing some licences issue in public headers
Hi Anthony, Thank you for doing this, it was much needed! Hi all, I think if we are going to commit this series for 4.17 then I would suggest to also commit patches 1-3 of my "introduce SPDX" series: https://marc.info/?l=xen-devel&m=16656522996 They are already acked/reviewed and are zero risk as they don't actually change any of the headers. For clarify, I don't mean to cause any trouble to this series. I am also happy to have this series committed in 4.17 without "introduce SPDX". Cheers, Stefano On Thu, 3 Nov 2022, Anthony PERARD wrote: > Patch series available in this git branch: > https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git > br.licences-fix-public-headers-v2 > > Hi, > > Andrew pointed out some licences issue: > > https://lore.kernel.org/xen-devel/b58f5340-d4fa-df9d-89de-6137005ad...@citrix.com/T/#u > tracked here: https://gitlab.com/xen-project/xen/-/issues/35 > > So I attempt to fix them with this series. > > For 4.17: > This mostly change "documentation" so little risk for those patch, except > "xen-foreign: Capture licences from the input headers" which changes > "mkheader.py" which could generate broken headers. > > Anthony PERARD (6): > xen: Add licence information to public/errno.h > xen: Used SPDX identifier in some public headers > tools/include/xen-foreign: Add SPDX identifier to generated headers > xen: Add licence header to device_tree_defs.h > Rework COPYING installed in /usr/include/xen/, due to several licences > xen: Used SPDX identifier in public headers > > tools/include/Makefile | 1 - > xen/include/public/arch-arm.h| 19 +- > xen/include/public/arch-arm/hvm/save.h | 19 +- > xen/include/public/arch-arm/smccc.h | 19 +- > xen/include/public/arch-x86/cpufeatureset.h | 19 +- > xen/include/public/arch-x86/cpuid.h | 19 +- > xen/include/public/arch-x86/guest-acpi.h | 19 +- > xen/include/public/arch-x86/hvm/save.h | 19 +- > xen/include/public/arch-x86/hvm/start_info.h | 19 +- > xen/include/public/arch-x86/pmu.h| 19 +- > xen/include/public/arch-x86/xen-mca.h| 19 +- > xen/include/public/arch-x86/xen-x86_32.h | 19 +- > xen/include/public/arch-x86/xen-x86_64.h | 19 +- > xen/include/public/arch-x86/xen.h| 19 +- > xen/include/public/arch-x86_32.h | 19 +- > xen/include/public/arch-x86_64.h | 19 +- > xen/include/public/argo.h| 19 +- > xen/include/public/callback.h| 19 +- > xen/include/public/device_tree_defs.h| 6 + > xen/include/public/dom0_ops.h| 19 +- > xen/include/public/domctl.h | 19 +- > xen/include/public/elfnote.h | 19 +- > xen/include/public/errno.h | 2 ++ > xen/include/public/event_channel.h | 19 +- > xen/include/public/features.h| 19 +- > xen/include/public/grant_table.h | 19 +- > xen/include/public/hvm/dm_op.h | 19 +- > xen/include/public/hvm/e820.h| 19 +- > xen/include/public/hvm/hvm_info_table.h | 19 +- > xen/include/public/hvm/hvm_op.h | 19 +- > xen/include/public/hvm/hvm_vcpu.h| 19 +- > xen/include/public/hvm/hvm_xs_strings.h | 19 +- > xen/include/public/hvm/ioreq.h | 19 +- > xen/include/public/hvm/params.h | 19 +- > xen/include/public/hvm/pvdrivers.h | 19 +- > xen/include/public/hvm/save.h| 19 +- > xen/include/public/hypfs.h | 19 +- > xen/include/public/io/9pfs.h | 19 +- > xen/include/public/io/blkif.h| 19 +- > xen/include/public/io/cameraif.h | 19 +- > xen/include/public/io/console.h | 19 +- > xen/include/public/io/displif.h | 19 +- > xen/include/public/io/fbif.h | 19 +- > xen/include/public/io/fsif.h | 19 +- > xen/include/public/io/kbdif.h| 19 +- > xen/include/public/io/libxenvchan.h | 19 +- > xen/include/public/io/netif.h| 19 +- > xen/include/public/io/pciif.h| 19 +- > xen/include/public/io/protocols.h| 19 +- > xen/include/public/io/pvcalls.h | 19 +- > xen/include/public/io/ring.h | 19 +--
[xen-unstable-smoke test] 174725: tolerable all pass - PUSHED
flight 174725 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/174725/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen 08c6f57cfebad4046dabc05092b4a27c61a39980 baseline version: xen a4180b03fffafa1868b0bcacc20198d4caef2908 Last test of basis 174699 2022-11-09 21:00:29 Z1 days Testing same since 174725 2022-11-10 20:00:24 Z0 days1 attempts People who touched revisions under test: Luca Fancellu jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git a4180b03ff..08c6f57cfe 08c6f57cfebad4046dabc05092b4a27c61a39980 -> smoke
Re: Proposal for virtual IOMMU binding b/w vIOMMU and passthrough devices
On Mon, 31 Oct 2022, Bertrand Marquis wrote: > Hi All, > > > On 30 Oct 2022, at 21:14, Stefano Stabellini wrote: > > > > On Sun, 30 Oct 2022, Julien Grall wrote: > >> Hi Stefano, > >> > >> On 30/10/2022 14:23, Stefano Stabellini wrote: > >>> On Fri, 28 Oct 2022, Julien Grall wrote: > On 28/10/2022 14:13, Bertrand Marquis wrote: > >> On 28 Oct 2022, at 14:06, Julien Grall wrote: > >> > >> Hi Rahul, > >> > >> On 28/10/2022 13:54, Rahul Singh wrote: > For ACPI, I would have expected the information to be > found in > the IOREQ. > > So can you add more context why this is necessary for > everyone? > >>> We have information for IOMMU and Master-ID but we don’t > >>> have > >>> information for linking vMaster-ID to pMaster-ID. > >> > >> I am confused. Below, you are making the virtual master ID > >> optional. So shouldn't this be mandatory if you really need > >> the > >> mapping with the virtual ID? > > vMasterID is optional if user knows pMasterID is unique on the > > system. But if pMasterId is not unique then user needs to > > provide > > the vMasterID. > > So the expectation is the user will be able to know that the > pMasterID > is uniq. This may be easy with a couple of SMMUs, but if you have > 50+ > (as suggested above). This will become a pain on larger system. > > IHMO, it would be much better if we can detect that in libxl (see > below). > >>> We can make the vMasterID compulsory to avoid complexity in libxl to > >>> solve this > >> > >> In general, complexity in libxl is not too much of problem. > >>> > >>> I agree with this and also I agree with Julien's other statement: > >>> > >>> "I am strongly in favor of libxl to modify it if it greatly improves the > >>> user experience." > >>> > >>> I am always in favor of reducing complexity for the user as they > >>> typically can't deal with tricky details such as MasterIDs. In general, > >>> I think we need more automation with our tooling. > >>> > >>> However, it might not be as simple as adding support for automatically > >>> generating IDs in libxl because we have 2 additional cases to support: > >>> 1) dom0less > >>> 2) statically built guests > >>> > >>> For 1) we would need the same support also in Xen? Which means more > >>> complexity in Xen. > >> Xen will need to parse the device-tree to find the mapping. So I am not > >> entirely convinced there will be more complexity needed other than > >> requiring a > >> bitmap to know which vMasterID has been allocated. > >> > >> That said, you would still need one to validate the input provided by the > >> user. So overall maybe there will be no added complexity? > >> > >>> > >>> 2) are guests like Zephyr that consume a device tree at > >>> build time instead of runtime. These guests are built specifically for a > >>> given environment and it is not a problem to rebuild them for every Xen > >>> release. > >>> > >>> However I think it is going to be a problem if we have to run libxl to > >>> get the device tree needed for the Zephyr build. That is because it > >>> means that the Zephyr build system would have to learn how to compile > >>> (or crosscompile) libxl in order to retrieve the data needed for its > >>> input. Even for systems based on Yocto (Yocto already knows how to build > >>> libxl) would cause issues because of internal dependencies this would > >>> introduce. > >> > >> That would not be very different to how this works today for Zephyr. They > >> need > >> libxl to generate the guest DT. > >> > >> That said, I agree this is a bit of a pain... > > > > Yeah.. > > > > > >>> So I think the automatic generation might be best done in another tool. > >> It sounds like what you want is creating something similar to libacpi but > >> for > >> Device-Tree. That should work with some caveats. > > > > Yes, something like that. We have a framework for reading, editing and > > generating Device Tree: Lopper https://github.com/devicetree-org/lopper > > > > It is mostly targeted at build time but it could also be invoked on > > target at runtime. > > > > > >>> I think we need something like a script that takes a partial device tree > >>> as input and provides a more detailed partial device tree as output with > >>> the generated IDs. > >> > >> AFAICT, having the partial device-tree is not enough. You also need the > >> real > >> DT to figure out the pMaster-ID. > >> > >>> > >>> If we did it that way, we could call the script from libxl, but also we > >>> could call it separately from ImageBuilder for dom0less and Zephyr/Yocto > >>> could also call it. > >>> > >>> Basically we make it easier for everyone to use it. The only price to > >>> pay is that it will be a bit less efficient
Re: [PATCH for-4.17 v2] hvm/apic: repurpose the reporting of the APIC assist options
On 04/11/2022 16:18, Roger Pau Monne wrote:
> The current reporting of the hardware assisted APIC options is done by
> checking "virtualize APIC accesses" which is not very helpful, as that
> feature doesn't avoid a vmexit, instead it does provide some help in
> order to detect APIC MMIO accesses in vmexit processing.
>
> Repurpose the current reporting of xAPIC assistance to instead report
> such feature as present when there's support for "TPR shadow" and
> "APIC register virtualization" because in that case some xAPIC MMIO
> register accesses are handled directly by the hardware, without
> requiring a vmexit.
>
> For symetry also change assisted x2APIC reporting to require
> "virtualize x2APIC mode" and "APIC register virtualization", dropping
> the option to also be reported when "virtual interrupt delivery" is
> available. Presence of the "virtual interrupt delivery" feature will
> be reported using a different option.
>
> Fixes: 2ce11ce249 ('x86/HVM: allow per-domain usage of hardware virtualized
> APIC')
> Signed-off-by: Roger Pau Monné
> ---
> I find the logic in vmx_vlapic_msr_changed() hard to follow, but I
> don't want to rewrite the function logic at this point.
> ---
> Changes since v1:
> - Fix Viridian MSR tip conditions.
> ---
> xen/arch/x86/hvm/viridian/viridian.c | 2 +-
> xen/arch/x86/hvm/vmx/vmcs.c | 8
> xen/arch/x86/hvm/vmx/vmx.c | 25 ++---
> xen/arch/x86/traps.c | 4 +---
> 4 files changed, 24 insertions(+), 15 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/viridian/viridian.c
> b/xen/arch/x86/hvm/viridian/viridian.c
> index 25dca93e8b..44eb3d0519 100644
> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -197,7 +197,7 @@ void cpuid_viridian_leaves(const struct vcpu *v, uint32_t
> leaf,
> res->a = CPUID4A_RELAX_TIMER_INT;
> if ( viridian_feature_mask(d) & HVMPV_hcall_remote_tlb_flush )
> res->a |= CPUID4A_HCALL_REMOTE_TLB_FLUSH;
> -if ( !cpu_has_vmx_apic_reg_virt )
> +if ( !has_assisted_xapic(d) )
> res->a |= CPUID4A_MSR_BASED_APIC;
This check is broken before and after. It needs to be keyed on
virtualised interrupt delivery, not register acceleration.
But doing this correctly needs a per-domain vintr setting, which we
don't have yet.
It is marginally less broken with this change, than without, but that's
not saying much.
> if ( viridian_feature_mask(d) & HVMPV_hcall_ipi )
> res->a |= CPUID4A_SYNTHETIC_CLUSTER_IPI;
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index a1aca1ec04..7bb96e1a8e 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -1136,7 +1136,7 @@ static int construct_vmcs(struct vcpu *v)
>
> if ( !has_assisted_xapic(d) )
> v->arch.hvm.vmx.secondary_exec_control &=
> -~SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
> +~SECONDARY_EXEC_APIC_REGISTER_VIRT;
>
> if ( cpu_has_vmx_secondary_exec_control )
> __vmwrite(SECONDARY_VM_EXEC_CONTROL,
> @@ -2156,10 +2156,10 @@ int __init vmx_vmcs_init(void)
> if ( !ret )
> {
> /* Check whether hardware supports accelerated xapic and x2apic. */
> -assisted_xapic_available = cpu_has_vmx_virtualize_apic_accesses;
> +assisted_xapic_available = cpu_has_vmx_tpr_shadow &&
> + cpu_has_vmx_apic_reg_virt;
> assisted_x2apic_available = cpu_has_vmx_virtualize_x2apic_mode &&
> -(cpu_has_vmx_apic_reg_virt ||
> - cpu_has_vmx_virtual_intr_delivery);
> +cpu_has_vmx_apic_reg_virt;
apic reg virt already depends on tpr shadow, so that part of the
condition is redundant.
virtualise x2apic mode and apic reg virt aren't dependent, but they do
only ever appear together in hardware.
Keeping the conditionals might be ok to combat a bad outer hypervisor,
but ...
> register_keyhandler('v', vmcs_dump, "dump VT-x VMCSs", 1);
> }
>
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index e624b415c9..bf0fe3355c 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3405,25 +3405,29 @@ static void vmx_install_vlapic_mapping(struct vcpu *v)
>
> void vmx_vlapic_msr_changed(struct vcpu *v)
> {
> +bool virtualize_x2apic_mode = has_assisted_x2apic(v->domain) ||
> + (cpu_has_vmx_virtualize_x2apic_mode &&
> + cpu_has_vmx_virtual_intr_delivery);
... this is still wrong, and ...
> struct vlapic *vlapic = vcpu_vlapic(v);
> unsigned int msr;
>
> -if ( !has_assisted_xapic(v->domain) &&
> - !has_assisted_x2apic(v->domain) )
> +if ( !cpu_has_vmx_virtualize_apic_accesses &&
> + !virtualize_x2apic_mode )
>
RE: [PATCH v6 00/11] xen/arm: Add Armv8-R64 MPU support to Xen - Part#1
On Mon, 7 Nov 2022, Wei Chen wrote: > Hi Julien, > > > -Original Message- > > From: Julien Grall > > Sent: 2022年11月7日 18:16 > > To: Wei Chen ; xen-devel@lists.xenproject.org > > Cc: nd ; Stefano Stabellini ; Bertrand > > Marquis ; Volodymyr Babchuk > > > > Subject: Re: [PATCH v6 00/11] xen/arm: Add Armv8-R64 MPU support to Xen - > > Part#1 > > > > > > > > On 07/11/2022 09:52, Wei Chen wrote: > > > Hi Julien, > > > > Hi, > > > > > > > >>>- Supports only a single Security state - Secure. > > >>>- MPU in EL1 & EL2 is configurable, MMU in EL1 is configurable. > > >>> > > >>> These patch series are implementing the Armv8-R64 MPU support > > >>> for Xen, which are based on the discussion of > > >>> "Proposal for Porting Xen to Armv8-R64 - DraftC" [1]. > > >>> > > >>> We will implement the Armv8-R64 and MPU support in three stages: > > >>> 1. Boot Xen itself to idle thread, do not create any guests on it. > > >> > > >> I read this as I can build Xen and see it boots (not creating domain). > > >> However... HAS_MPU is not defined and I was expecting mm.c to be > > >> modified to cater the MPU support. So I am a bit ensure what the series > > >> is actually doing. > > >> > > > > > > These 11 patches are part#1 of stage#1, the full stage#1 has about 30 > > > patches. We have some concerns if we send so many patches at once, the > > > review pressure of maintainers may be very high, so we only choose about > > > 10 to send as part of it. But this also means that we can't do a > > relatively > > > complete thing in this part#1 series. > > > > > > We want to hear some suggestions from you to make so many patches can be > > > Reviewed efficiently. Can we send the patches by stages, even the > > stage#1 > > > will have about 30 patches? > > > > 30 patches in a go is no too bad. I would personally prefer that because > > at least I have better idea of the shape of the code after stage#1 and > > also possibly test it (I need to check if I have access for the ARMv8-R > > model). > > > > I also prefer to this way. After we have addressed the comments in > this series, we will send the full stage#1 patches together in v2. One suggestion to make things easier to review and to commit is to organize the series in a way so that the first 10 patches can still be committed first independently, even if all 30 patches are sent together. Or alternatively only send 10 patches but also add a link to a github/gitlab tree with all the 30+ patches so that maintainers can have a look how the whole work fit together. I think we are all on the same page -- I just wanted to highlight that we don't have to finish the review of all 30 patches before we can start committing some of the initial patches in the series. Cheers, Stefano
Re: [PATCH v6 05/11] xen/arm: define Xen start address for FVP BaseR platform
On Wed, 9 Nov 2022, Julien Grall wrote: > > > -Original Message- > > > From: Julien Grall > > > Sent: 2022年11月7日 3:20 > > > To: Wei Chen ; xen-devel@lists.xenproject.org > > > Cc: nd ; Stefano Stabellini ; > > > Bertrand > > > Marquis ; Volodymyr Babchuk > > > ; Jiamei Xie > > > Subject: Re: [PATCH v6 05/11] xen/arm: define Xen start address for FVP > > > BaseR platform > > > > > > > > > > > > On 04/11/2022 10:07, Wei Chen wrote: > > > > On Armv8-A, Xen has a fixed virtual start address (link address > > > > too) for all Armv8-A platforms. In an MMU based system, Xen can > > > > map its loaded address to this virtual start address. So, on > > > > Armv8-A platforms, the Xen start address does not need to be > > > > configurable. But on Armv8-R platforms, there is no MMU to map > > > > loaded address to a fixed virtual address and different platforms > > > > will have very different address space layout. So Xen cannot use > > > > a fixed physical address on MPU based system and need to have it > > > > configurable. > > > > > > > > So in this patch, we reuse the existing arm/platforms to store > > > > Armv8-R platforms' parameters. And `XEN_START_ADDRESS` is one > > > > kind of FVP BaseR platform's parameters. So we define default > > > > `XEN_START_ADDRESS` for FVP BaseR in its platform file. > > > > > > > > We also introduce one Kconfig option for users to override the > > > > default Xen start address of selected platform, if they think > > > > the default address doesn't suit their scenarios. For this > > > > Kconfig option, we use an unaligned address "0x" as the > > > > default value to indicate that users haven't used a customized > > > > Xen start address. > > > > > > > > And as we introduced Armv8-R platforms to Xen, that means the > > > > existed Arm64 platforms should not be listed in Armv8-R platform > > > > list, so we add !ARM_V8R dependency for these platforms. > > > > > > > > Signed-off-by: Wei Chen > > > > Signed-off-by: Jiamei.Xie > > > > --- > > > >xen/arch/arm/Kconfig | 11 +++ > > > >xen/arch/arm/include/asm/platforms/fvp_baser.h | 14 ++ > > > > > > I looked at the content of fvp_baser.h after this series is applied. > > > There are a bit of boiler plate that I expect to be part for other > > > platforms. In particular... > > > > > > >xen/arch/arm/platforms/Kconfig | 16 +--- > > > >3 files changed, 38 insertions(+), 3 deletions(-) > > > >create mode 100644 xen/arch/arm/include/asm/platforms/fvp_baser.h > > > > > > > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig > > > > index ad592367bd..ac276307d6 100644 > > > > --- a/xen/arch/arm/Kconfig > > > > +++ b/xen/arch/arm/Kconfig > > > > @@ -138,6 +138,17 @@ config TEE > > > > This option enables generic TEE mediators support. It allows > > > guests > > > > to access real TEE via one of TEE mediators implemented in > > > > XEN. > > > > > > > > +config XEN_START_ADDRESS > > > > + hex "Xen start address: keep default to use platform defined > > > address" > > > > + default 0x > > > > > > ... this default value will need to be tested everywhere. At least for > > > now, I think you can avoid the per platform header by using the Kconfig > > > to select the proper address (see the config for selecting early printk > > > address). > > > > > > This will also avoids to use an invalid value here. > > > > > > > We had considered to use Kconfig to define the start addresses of v8R64 > > platforms (prompt users to input the address). But we also want to provide > > a default start address for each platform (Discussed in [1], header for > > default value, Kconfig option for customized address). > Why do you want to provide a default value? And how it is guaranteed that it > will work for most of the users? > > > > > We also had thought to use Kconfig to define a default start address > > for each platform like what we had done for early printk in RFC[2]. > > But this method has been deprecated. > > Most of the current Xen is board agnostic except the UART. We push back the > addition of new one because the address can be found in the firmware table and > I wanted to avoid increase the number of option (there are dozens of platform > out...). > > > > > So if we don’t use header files, just use the Kconfig, we can't > > provide a default start address for platforms, and have to force users > > to enter the start address. > > I am not sure I see the problem to force the user to enter the start address. > My worry with per-platform default value is we end up to force each vendor to > provide an header in order to boot Xen. > > I think it would be better to provide a config tailored for that platform > (whether it is part of Xen can be debatable). This would allow a user to try a > release Xen on their platform with zero changes in the code. I agree with Julien, especially on t
[linux-5.4 test] 174704: regressions - FAIL
flight 174704 linux-5.4 real [real] http://logs.test-lab.xenproject.org/osstest/logs/174704/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-credit2 18 guest-start/debian.repeat fail REGR. vs. 174540 Tests which are failing intermittently (not blocking): test-amd64-i386-libvirt-xsm 8 xen-boot fail in 174638 pass in 174704 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-install fail in 174638 pass in 174704 test-amd64-i386-pair 11 xen-install/dst_host fail in 174638 pass in 174704 test-armhf-armhf-xl-rtds 14 guest-start fail in 174638 pass in 174704 test-armhf-armhf-xl-credit1 19 guest-start.2fail in 174646 pass in 174638 test-amd64-i386-qemuu-rhel6hvm-amd 12 redhat-install fail in 174646 pass in 174704 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 12 debian-hvm-install fail in 174646 pass in 174704 test-armhf-armhf-libvirt-qcow2 13 guest-startfail in 174646 pass in 174704 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-install fail in 174672 pass in 174704 test-armhf-armhf-xl-credit2 14 guest-start fail in 174672 pass in 174704 test-armhf-armhf-xl-multivcpu 18 guest-start/debian.repeat fail in 174684 pass in 174672 test-armhf-armhf-xl-credit1 14 guest-start fail in 174684 pass in 174704 test-armhf-armhf-xl-credit1 18 guest-start/debian.repeat fail pass in 174646 test-armhf-armhf-xl-multivcpu 14 guest-start fail pass in 174684 test-armhf-armhf-libvirt-raw 13 guest-startfail pass in 174684 test-armhf-armhf-xl-rtds 18 guest-start/debian.repeat fail pass in 174684 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-raw 15 saverestore-support-check fail in 174684 like 174540 test-armhf-armhf-xl-multivcpu 15 migrate-support-check fail in 174684 never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-check fail in 174684 never pass test-armhf-armhf-libvirt-raw 14 migrate-support-check fail in 174684 never pass test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 174540 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 174540 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 174540 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 174540 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 174540 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 174540 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 174540 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 174540 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 174540 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 174540 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 174540 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-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-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-amd64-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-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 migra
[linux-linus test] 174703: regressions - FAIL
flight 174703 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/174703/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-xl-xsm 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-credit1 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-credit2 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-libvirt-raw 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl-arndale 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl-vhd 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-examine 8 reboot fail REGR. vs. 173462 test-armhf-armhf-xl-credit2 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl-credit1 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-examine 8 reboot fail REGR. vs. 173462 test-arm64-arm64-xl-vhd 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-libvirt-xsm 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-seattle 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-libvirt-qcow2 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl-multivcpu 8 xen-bootfail REGR. vs. 173462 test-armhf-armhf-libvirt-raw 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-libvirt 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl 8 xen-boot fail REGR. vs. 173462 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds 8 xen-boot fail REGR. vs. 173462 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 173462 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 173462 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 173462 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 173462 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 173462 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass version targeted for testing: linuxf67dd6ce0723ad013395f20a3f79d8a437d3f455 baseline version: linux9d84bb40bcb30a7fa16f33baa967aeb9953dda78 Last test of basis 173462 2022-10-07 18:41:45 Z 34 days Failing since173470 2022-10-08 06:21:34 Z 33 days 53 attempts Testing same since 174703 2022-11-10 02:10:19 Z0 days1 attempts 1603 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-xl pass test-amd64-coresched-amd64-xlpass test-arm64-arm64-xl fail test-armhf-armhf-xl fail test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-am
Re: Xen Arm vpl011 UART will cause segmentation fault in Linux guest
On Wed, 9 Nov 2022, Michal Orzel wrote: > Hi Jiamei, > > On 09/11/2022 09:25, Jiamei Xie wrote: > > > > > > Hi Michal， > > > > Below log can be got when stating the linux guest. It says 9c09 is sbsa. > > And 9c09 is also output > > in bootlogd error message: > > Serial: AMBA PL011 UART driver > > 9c0b.uart: ttyAMA0 at MMIO 0x9c0b (irq = 12, base_baud = 0) is a > > PL011 rev2 > > printk: console [ttyAMA0] enabled > > 9c09.sbsa-uart: ttyAMA1 at MMIO 0x9c09 (irq = 15, base_baud = 0) is > > a SBSA > > > > Xen behavior is correct and this would be Linux fault to try to write to > DMACR for SBSA UART device. > DMACR is just an example. If you try to program e.g. the baudrate (through > LCR) for VPL011 it will > also result in injecting abort into the guest. Should Xen support it? No. The > reason why is that > it is not spec compliant operation. SBSA specification directly specifies > what registers are exposed. > If Linux tries to write to some of the none-spec compliant registers - it is > its fault. Yeah, we need to fix Linux. FYI this is not the first bug in Linux affecting the sbsa-uart driver: the issue is that the pl011 driver and the sbsa-uart driver share the same code in Linux so it happens sometimes that a pl011-only feature creeps into the sbsa-uart driver by mistake. > >> -Original Message- > >> From: Michal Orzel > >> Sent: Wednesday, November 9, 2022 3:40 PM > >> To: Jiamei Xie ; xen-devel@lists.xenproject.org > >> Cc: Wei Chen ; Bertrand Marquis > >> ; jul...@xen.org; sstabell...@kernel.org > >> Subject: Re: Xen Arm vpl011 UART will cause segmentation fault in Linux > >> guest > >> > >> Hi Jiamei, > >> > >> On 09/11/2022 08:20, Jiamei Xie wrote: > >>> > >>> > >>> Hi all, > >>> > >>> When the guest kernel enables DMA engine with > >> "CONFIG_DMA_ENGINE=y", Linux AMBA PL011 driver will access PL011 > >> DMACR register. But this register have not been supported by vpl011 of Xen. > >> Xen will inject a data abort into guest, this will cause segmentation > >> fault of > >> guest with the below message: > >> I am quite confused. > >> VPL011 implements SBSA UART which only implements some subset of PL011 > >> operations (SBSA UART is not PL011). > >> According to spec (SBSA ver. 6.0), the SBSA_UART does not support DMA > >> features so Xen code is fine. > >> When Xen exposes vpl011 device to a guest, this device has "arm,sbsa-uart" > >> compatible and not "uart-pl011". > >> Linux driver "amba-pl011.c" should see this compatible and assign proper > >> operations (sbsa_uart_pops instead of amba_pl011_pops) that do not enable > >> DMA. > >> Maybe the issue is with your configuration? > >> > >> ~Michal > >> > >>> Unhandled fault at 0xffc00944d048 > >>> Mem abort info: > >>> ESR = 0x9600 > >>> EC = 0x25: DABT (current EL), IL = 32 bits > >>> SET = 0, FnV = 0 > >>> EA = 0, S1PTW = 0 > >>> FSC = 0x00: ttbr address size fault > >>> Data abort info: > >>> ISV = 0, ISS = 0x > >>> CM = 0, WnR = 0 > >>> swapper pgtable: 4k pages, 39-bit VAs, pgdp=20e2e000 > >>> [ffc00944d048] pgd=10003803, p4d=10003803, > >> pud=10003803, pmd=10003fffa803, pte=00689c090f13 > >>> Internal error: ttbr address size fault: 9600 [#1] PREEMPT SMP > >>> Modules linked in: > >>> CPU: 0 PID: 132 Comm: bootlogd Not tainted 5.15.44-yocto-standard #1 > >>> pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > >>> pc : pl011_stop_rx+0x70/0x80 > >>> lr : uart_tty_port_shutdown+0x44/0x110 > >>> sp : ffc00999bba0 > >>> x29: ffc00999bba0 x28: ff80234ac380 x27: ff8022f5d000 > >>> x26: x25: 45585401 x24: > >>> x23: ff8021ba4660 x22: 0001 x21: ff8021a0e2a0 > >>> x20: ff802198f880 x19: ff8021a0e1a0 x18: > >>> x17: x16: x15: > >>> x14: x13: x12: > >>> x11: x10: x9 : ffc00871ba14 > >>> x8 : ffc0099de260 x7 : ff8021a0e318 x6 : 0003 > >>> x5 : ffc009315f20 x4 : ffc00944d038 x3 : > >>> x2 : ffc00944d048 x1 : x0 : 0048 > >>> Call trace: > >>> pl011_stop_rx+0x70/0x80 > >>> tty_port_shutdown+0x7c/0xb4 > >>> tty_port_close+0x60/0xcc > >>> uart_close+0x34/0x8c > >>> tty_release+0x144/0x4c0 > >>> __fput+0x78/0x220 > >>> fput+0x1c/0x30 > >>> task_work_run+0x88/0xc0 > >>> do_notify_resume+0x8d0/0x123c > >>> el0_svc+0xa8/0xc0 > >>> el0t_64_sync_handler+0xa4/0x130 > >>> el0t_64_sync+0x1a0/0x1a4 > >>> Code: b983 b901f001 794038a0 8b42 (b941) > >>> ---[ end trace 83dd93df15c3216f ]--- > >>> note: bootlogd[132] exited with preempt_count 1 > >>> /etc/rcS.d/S07bootlogd: line 47: 132 Segmentation fault start-stop- > >> daemon > >>> In Xen, vpl011_mmio_write doesn't handle DMACR . And kernel doesn't > >> check if pl011_write
Re: [PATCH v3 0/4] Yocto Gitlab CI
On Thu, 10 Nov 2022, Michal Orzel wrote: > Hi Stefano, > > On 10/11/2022 01:18, Stefano Stabellini wrote: > > > > > > On Mon, 7 Nov 2022, Michal Orzel wrote: > >> Hi Bertrand and Stefano, > >> > >> On 31/10/2022 16:00, Bertrand Marquis wrote: > >>> > >>> > >>> Hi Michal, > >>> > On 31 Oct 2022, at 14:39, Michal Orzel wrote: > > Hi Bertrand, > > On 31/10/2022 15:00, Bertrand Marquis wrote: > > > > > > This patch series is a first attempt to check if we could use Yocto in > > gitlab ci to build and run xen on qemu for arm, arm64 and x86. > > > > The first patch is creating a container with all elements required to > > build Yocto, a checkout of the yocto layers required and an helper > > script to build and run xen on qemu with yocto. > > > > The second patch is creating containers with a first build of yocto done > > so that susbsequent build with those containers would only rebuild what > > was changed and take the rest from the cache. > > > > The third patch is adding a way to easily clean locally created > > containers. > > > > This is is mainly for discussion and sharing as there are still some > > issues/problem to solve: > > - building the qemu* containers can take several hours depending on the > > network bandwith and computing power of the machine where those are > > created > This is not really an issue as the build of the containers occurs on the > local > machines before pushing them to registry. Also, building the containers > will only be required for new Yocto releases. > > > - produced containers containing the cache have a size between 8 and > > 12GB depending on the architecture. We might need to store the build > > cache somewhere else to reduce the size. If we choose to have one > > single image, the needed size is around 20GB and we need up to 40GB > > during the build, which is why I splitted them. > > - during the build and run, we use a bit more then 20GB of disk which is > > over the allowed size in gitlab > As we could see during v2 testing, we do not have any space restrictions > on the Xen GitLab and I think we already decided to have the Yocto > integrated into our CI. > >>> > >>> Right, I should have modified this chapter to be coherent with your > >>> latest tests. > >>> Sorry for that. > >>> > > I will do some testing and get back to you with results + review. > >> I did some testing and here are the results: > >> > >> In the current form this series will fail when running CI because the > >> Yocto containers > >> are based on "From ubuntu:22.04" (there is no platform prefix), which > >> means that the containers > >> are built for the host architecture (in my case and in 99% of the cases of > >> the local build it will > >> be x86). In Gitlab we have 2 runners (arm64 and x86_64). This means that > >> all the test jobs would need > >> to specify x86_64 as a tag when keeping the current behavior. > >> After I built all the containers on my x86 machine, I pushed them to > >> registry and the pipeline was successful: > >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com%2Fxen-project%2Fpeople%2Fmorzel%2Fxen-orzelmichal%2F-%2Fpipelines%2F686853939&data=05%7C01%7Cmichal.orzel%40amd.com%7C2449f063e67341c3b95a08dac2b112a5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638036363027707274%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=EwTJrW2vuwQIugKc7mnzG9NNbsYLP6tw5UODzBMmPEE%3D&reserved=0 > > > > When I tested the previous version of this series I built the > > containers natively on ARM64, so that is also an option. > > > > > >> Here is the diff on patch no. 3 to make the series work (using x86 tag and > >> small improvement to include needs: []): > >> ``` > >> diff --git a/automation/gitlab-ci/test.yaml > >> b/automation/gitlab-ci/test.yaml > >> index 5c620fefce59..52cccec6f904 100644 > >> --- a/automation/gitlab-ci/test.yaml > >> +++ b/automation/gitlab-ci/test.yaml > >> @@ -65,6 +65,9 @@ > >> paths: > >>- 'logs/*' > >> when: always > >> + needs: [] > >> + tags: > >> +- x86_64 > >> > >> # Test jobs > >> build-each-commit-gcc: > >> @@ -206,19 +209,13 @@ yocto-qemuarm64: > >>extends: .yocto-test > >>variables: > >> YOCTO_BOARD: qemuarm64 > >> - tags: > >> -- arm64 > >> > >> yocto-qemuarm: > >>extends: .yocto-test > >>variables: > >> YOCTO_BOARD: qemuarm > >> - tags: > >> -- arm32 > >> > >> yocto-qemux86-64: > >>extends: .yocto-test > >>variables: > >> YOCTO_BOARD: qemux86-64 > >> - tags: > >> -- x86_64 > >> ``` > >> > >> Now, the logical way would be to build x86 yocto container for x86, arm64 > >> for arm64 and arm32 on arm64 or x86. > >> I tried building the container qemuarm64 specifying target arm64
Re: [PATCH v2 1/2] x86: Check return values from early_memremap calls
On 11/10/22 11:07, Dave Hansen wrote:
On 11/10/22 07:45, Ross Philipson wrote:
dt = early_memremap(initial_dtb, map_len);
+ if (!dt) {
+ pr_warn("failed to memremap initial dtb\n");
+ return;
+ }
Are all of these new pr_warn/err()'s really adding much value? They all
look pretty generic. It makes me wonder if we should just spit out a
generic message in early_memremap() and save all the callers the trouble.
These changes were prompted by some comments on an earlier patch set I
sent. It was requested that I fix the other missing checks for NULL
returns from these functions but I thought that was out of scope for
that patch set. So I agreed to submit this set and add the checks making
things consistent.
Oh, and don't we try to refer to functions() with parenthesis?
Yes I can fix that.
Thanks
Ross
Re: [PATCH v3] platform/x86: don't unconditionally attach Intel PMC when virtualized
On Thu, Nov 10, 2022 at 05:31:44PM +0100, Roger Pau Monne wrote:
> The current logic in the Intel PMC driver will forcefully attach it
> when detecting any CPU on the intel_pmc_core_platform_ids array,
> even if the matching ACPI device is not present.
>
> There's no checking in pmc_core_probe() to assert that the PMC device
> is present, and hence on virtualized environments the PMC device
> probes successfully, even if the underlying registers are not present.
> Previous to 21ae43570940 the driver would check for the presence of a
> specific PCI device, and that prevented the driver from attaching when
> running virtualized.
>
> Fix by only forcefully attaching the PMC device when not running
> virtualized. Note that virtualized platforms can still get the device
> to load if the appropriate ACPI device is present on the tables
> provided to the VM.
>
> Make an exception for the Xen initial domain, which does have full
> hardware access, and hence can attach to the PMC if present.
Reviewed-by: Andy Shevchenko
> Fixes: 21ae43570940 ('platform/x86: intel_pmc_core: Substitute PCI with CPUID
> enumeration')
> Signed-off-by: Roger Pau Monné
> Acked-by: David E. Box
> ---
> Changes since v2:
> - Don't split condition line.
>
> Changes since v1:
> - Use cpu_feature_enabled() instead of boot_cpu_has().
> ---
> drivers/platform/x86/intel/pmc/pltdrv.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/platform/x86/intel/pmc/pltdrv.c
> b/drivers/platform/x86/intel/pmc/pltdrv.c
> index 15ca8afdd973..ddfba38c2104 100644
> --- a/drivers/platform/x86/intel/pmc/pltdrv.c
> +++ b/drivers/platform/x86/intel/pmc/pltdrv.c
> @@ -18,6 +18,8 @@
> #include
> #include
>
> +#include
> +
> static void intel_pmc_core_release(struct device *dev)
> {
> kfree(dev);
> @@ -53,6 +55,13 @@ static int __init pmc_core_platform_init(void)
> if (acpi_dev_present("INT33A1", NULL, -1))
> return -ENODEV;
>
> + /*
> + * Skip forcefully attaching the device for VMs. Make an exception for
> + * Xen dom0, which does have full hardware access.
> + */
> + if (cpu_feature_enabled(X86_FEATURE_HYPERVISOR) &&
> !xen_initial_domain())
> + return -ENODEV;
> +
> if (!x86_match_cpu(intel_pmc_core_platform_ids))
> return -ENODEV;
>
> --
> 2.37.3
>
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 2/2] x86: Check return values from early_ioremap calls
On 11/10/22 13:07, Peter Zijlstra wrote: On Thu, Nov 10, 2022 at 03:45:21PM +, Ross Philipson wrote: On allocation failures, panic() was used since this seemed to be the action taken on other failures in the modules touched by this patch. How is the panic() more useful than the obvious NULL deref that also splats? My answer here is basically the same as the answer in the reply to Dave Hansen I sent a moment ago. I think one of the primary motivation was to make things consistent. Thanks Ross
[xen-unstable test] 174701: tolerable FAIL - PUSHED
flight 174701 xen-unstable real [real] flight 174723 xen-unstable real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/174701/ http://logs.test-lab.xenproject.org/osstest/logs/174723/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-freebsd10-amd64 7 xen-install fail pass in 174723-retest Tests which did not succeed, but are not blocking: test-amd64-i386-pair 10 xen-install/src_host fail like 174682 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 174682 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 174682 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 174682 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 174682 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 174682 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 174682 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 174682 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 174682 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 174682 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 174682 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 174682 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 174682 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 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-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail ne
Re: [PATCH for 4.17] arm: fix Kconfig symbol dependency on arm features
Hi Bertrand,
On 10/11/2022 08:46, Bertrand Marquis wrote:
On 9 Nov 2022, at 14:04, Luca Fancellu wrote:
The commit 3c2a14ea81c7 is introducing some unsupported arm features
that by default are disabled and are used for the cpufeature.c code.
As they are disabled by default, a typo in the Kconfig symbol they
depend on has landed in the codebase unnoticed, instead of depending
on ARM64 which does not exist, fix the code to depend on ARM_64 that
is the intended symbol.
Fixes: 3c2a14ea81c7 ("arm: Define kconfig symbols used by arm64 cpufeatures")
Signed-off-by: Luca Fancellu
Reviewed-by: Bertrand Marquis
I think this should go in 4.17 as it is fixing an invalid depends in Kconfig.
The change cannot create any issue as those config options are hidden and
default to n at the moment.
Committed.
Cheers,
--
Julien Grall
Re: [XEN v3] xen/arm: Enforce alignment check in debug build for {read, write}_atomic
Hi,
On 10/11/2022 00:00, Stefano Stabellini wrote:
On Tue, 8 Nov 2022, Ayan Kumar Halder wrote:
From: Ayan Kumar Halder
Xen provides helper to atomically read/write memory (see {read,
write}_atomic()). Those helpers can only work if the address is aligned
to the size of the access (see B2.2.1 ARM DDI 08476I.a).
On Arm32, the alignment is already enforced by the processor because
HSCTLR.A bit is set (it enforce alignment for every access). For Arm64,
this bit is not set because memcpy()/memset() can use unaligned access
for performance reason (the implementation is taken from the Cortex
library).
To avoid any overhead in production build, the alignment will only be
checked using an ASSERT. Note that it might be possible to do it in
production build using the acquire/exclusive version of load/store. But
this is left to a follow-up (if wanted).
Signed-off-by: Ayan Kumar Halder
Signed-off-by: Julien Grall
Reviewed-by: Michal Orzel
Reviewed-by: Bertrand Marquis
Acked-by: Stefano Stabellini
I have pushed this patch in a branch for-next/4.18 on my public repo. I
will apply the patch to staging once the tree re-opened.
Cheers,
--
Julien Grall
[[PATCH for-4.17 v1]] tools/ocaml/xenstored/xenstored.ml: fix incorrect scope
A debug statement got introduced and code not reindented
(as it was part of a security fix and was trying to avoid that),
however that resulted in *only* the debug statement being part of the 'if',
and everything else outside of it.
This results in some unnecessary ring checks for domains which otherwise
have IO credit.
Remove the debug line.
Fixes: 42f0581a91 ("tools/oxenstored: Implement live update for socket
connections")
Signed-off-by: Edwin Török
---
Reason for inclusion in 4.17:
- bugfix for commit already in master
Changes since v3:
- new in v4
---
tools/ocaml/xenstored/xenstored.ml | 1 -
1 file changed, 1 deletion(-)
diff --git a/tools/ocaml/xenstored/xenstored.ml
b/tools/ocaml/xenstored/xenstored.ml
index ffd43a4eee..79f04178d8 100644
--- a/tools/ocaml/xenstored/xenstored.ml
+++ b/tools/ocaml/xenstored/xenstored.ml
@@ -476,7 +476,6 @@ let _ =
let ring_scan_checker dom =
(* no need to scan domains already marked as for processing *)
if not (Domain.get_io_credit dom > 0) then
- debug "Looking up domid %d" (Domain.get_id dom);
let con = Connections.find_domain cons (Domain.get_id
dom) in
if not (Connection.has_more_work con) then (
Process.do_output store cons domains con;
--
2.34.1
Re: [PATCH v2 2/2] x86: Check return values from early_ioremap calls
On Thu, Nov 10, 2022 at 03:45:21PM +, Ross Philipson wrote: > On allocation failures, panic() was used since this seemed > to be the action taken on other failures in the modules > touched by this patch. How is the panic() more useful than the obvious NULL deref that also splats?
[PATCH 3/3] x86/pci: Fix racy accesses to MSI-X Control register
Concurrent access the the MSI-X control register are not serialized
with a suitable lock. For example, in msix_capability_init() access
use the pcidevs_lock() but some calls to msi_set_mask_bit() use the
interrupt descriptor lock.
This can lead to MSI-X being incorrectly disabled and subsequent
failures due to msix_memory_decoded() calls that check for MSI-X being
enabled.
This was seen with some non-compliant hardware that gated MSI-X
messages on the per-vector mask bit only (i.e., the MSI-X Enable bit
and Function Mask bits in the MSI-X Control register were ignored). An
interrupt (with a pending move) for vector 0 would occur while vector
1 was being initialized in msix_capability_init(). Updates the the
Control register would race and the vector 1 initialization would
intermittently fail with -ENXIO.
Typically a race between initializing a vector and another vector
being moved doesn't occur because:
1. Racy Control accesses only occur when MSI-X is (guest) disabled
2 Hardware should only raise interrupts when MSI-X is enabled and unmasked.
3. Xen always sets Function Mask when temporarily enabling MSI-X.
But there may be other race conditions depending on hardware and guest
driver behaviour (e.g., disabling MSI-X when a IRQ has a pending move
on another PCPU).
Fix this by:
1. Tracking the host and guest enable state in a similar way to the
host and guest maskall state. Note that since multiple CPUs can be
updating different vectors concurrently, a counter is needed for
the host enable state.
2. Add a new lock for serialize the Control read/modify/write
sequence.
3. Wrap the above in two helper functions (msix_update_lock(), and
msix_update_unlock()), which bracket any MSI-X register updates
that require MSI-X to be (temporarily) enabled.
Signed-off-by: David Vrabel
SIM: https://t.corp.amazon.com/P63914633
CR: https://code.amazon.com/reviews/CR-79020945
---
xen/arch/x86/include/asm/msi.h | 3 +
xen/arch/x86/msi.c | 215 +
xen/drivers/passthrough/msi.c | 1 +
3 files changed, 114 insertions(+), 105 deletions(-)
diff --git a/xen/arch/x86/include/asm/msi.h b/xen/arch/x86/include/asm/msi.h
index fe670895ee..aa36e44f4e 100644
--- a/xen/arch/x86/include/asm/msi.h
+++ b/xen/arch/x86/include/asm/msi.h
@@ -237,7 +237,10 @@ struct arch_msix {
int table_refcnt[MAX_MSIX_TABLE_PAGES];
int table_idx[MAX_MSIX_TABLE_PAGES];
spinlock_t table_lock;
+spinlock_t control_lock;
bool host_maskall, guest_maskall;
+uint16_t host_enable;
+bool guest_enable;
domid_t warned;
};
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 6c675d11d1..8e394da07a 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -147,6 +147,57 @@ static bool msix_memory_decoded(const struct pci_dev *dev,
unsigned int pos)
return memory_decoded(dev);
}
+
+/*
+ * Ensure MSI-X interrupts are masked during setup. Some devices require
+ * MSI-X to be enabled before we can touch the MSI-X registers. We need
+ * to mask all the vectors to prevent interrupts coming in before they're
+ * fully set up.
+ */
+static uint16_t msix_update_lock(struct pci_dev *dev, unsigned int pos)
+{
+uint16_t control, new_control;
+unsigned long flags;
+
+spin_lock_irqsave(&dev->msix->control_lock, flags);
+
+dev->msix->host_enable++;
+
+control = pci_conf_read16(dev->sbdf, msix_control_reg(pos));
+if ( !(control & PCI_MSIX_FLAGS_ENABLE) )
+{
+new_control = control | PCI_MSIX_FLAGS_ENABLE | PCI_MSIX_FLAGS_MASKALL;
+pci_conf_write16(dev->sbdf, msix_control_reg(pos), new_control);
+}
+else
+dev->msix->guest_enable = true;
+
+spin_unlock_irqrestore(&dev->msix->control_lock, flags);
+
+return control;
+}
+
+static void msix_update_unlock(struct pci_dev *dev, unsigned int pos, uint16_t
control)
+{
+uint16_t new_control;
+unsigned long flags;
+
+spin_lock_irqsave(&dev->msix->control_lock, flags);
+
+dev->msix->host_enable--;
+
+new_control = control & ~(PCI_MSIX_FLAGS_ENABLE | PCI_MSIX_FLAGS_MASKALL);
+
+if ( dev->msix->host_enable || dev->msix->guest_enable )
+new_control |= PCI_MSIX_FLAGS_ENABLE;
+if ( dev->msix->host_maskall || dev->msix->guest_maskall ||
dev->msix->host_enable )
+new_control |= PCI_MSIX_FLAGS_MASKALL;
+if ( new_control != control )
+pci_conf_write16(dev->sbdf, msix_control_reg(pos), control);
+
+spin_unlock_irqrestore(&dev->msix->control_lock, flags);
+}
+
/*
* MSI message composition
*/
@@ -288,7 +339,7 @@ static void msi_set_enable(struct pci_dev *dev, int enable)
__msi_set_enable(seg, bus, slot, func, pos, enable);
}
-static void msix_set_enable(struct pci_dev *dev, int enable)
+static void msix_force_disable(struct pci_dev *dev)
{
int pos;
u16 control, seg = dev->seg;
@@ -299,11 +350,16 @@ static void msix_set_enable(struct pci_dev *dev, int
enable)
pos = pci_
[PATCH 0/3] x86: Fix racy accesses to MSI-X Control register
The main patch in this series is 3/3 with some preparatory patches to simplify the implementation. To summarize: Concurrent access the the MSI-X control register are not serialized with a suitable lock. For example, in msix_capability_init() access use the pcidevs_lock() but some calls to msi_set_mask_bit() use the interrupt descriptor lock. This can lead to MSI-X being incorrectly disabled and subsequent failures due to msix_memory_decoded() calls that check for MSI-X being enabled. David
[PATCH 1/3] x86/msi: consistently handle BAR mapping failures in MSI-X setup
When setting up an MSI-X vector in msix_capability_init() the error
handling after a BAR mapping failure is different depending on whether
the first page fails or a subsequent page. There's no reason to break
working vectors so consistently use the later error handling
behaviour.
The zap_on_error flag was added as part of XSA-337, beb54596cfda
(x86/MSI-X: restrict reading of table/PBA bases from BARs), but
appears to be unrelated to XSA-337 and is not useful because:
1. table.first and pba.first are not used unless msix->used_vectors > 0.
2. Force disabling MSI-X in this error path is not necessary as the
per-vector mask is still still set.
Signed-off-by: David Vrabel
CR: https://code.amazon.com/reviews/CR-79020908
---
xen/arch/x86/msi.c | 12 +---
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index d0bf63df1d..8bde6b9be1 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -776,7 +776,7 @@ static int msix_capability_init(struct pci_dev *dev,
u8 bus = dev->bus;
u8 slot = PCI_SLOT(dev->devfn);
u8 func = PCI_FUNC(dev->devfn);
-bool maskall = msix->host_maskall, zap_on_error = false;
+bool maskall = msix->host_maskall;
unsigned int pos = pci_find_cap_offset(seg, bus, slot, func,
PCI_CAP_ID_MSIX);
@@ -875,8 +875,6 @@ static int msix_capability_init(struct pci_dev *dev,
BITS_TO_LONGS(msix->nr_entries) - 1);
WARN_ON(rangeset_overlaps_range(mmio_ro_ranges, msix->pba.first,
msix->pba.last));
-
-zap_on_error = true;
}
else if ( !msix->table.first )
{
@@ -897,14 +895,6 @@ static int msix_capability_init(struct pci_dev *dev,
if ( idx < 0 )
{
-if ( zap_on_error )
-{
-msix->table.first = 0;
-msix->pba.first = 0;
-
-control &= ~PCI_MSIX_FLAGS_ENABLE;
-}
-
pci_conf_write16(dev->sbdf, msix_control_reg(pos), control);
xfree(entry);
return idx;
--
2.30.2
[PATCH 2/3] x86/msi: remove return value from msi_set_mask_bit()
The return value was only used for WARN()s or BUG()s so it has no
functional purpose. Simplify the code by removing it.
The meaning of the return value and the purpose of the various WARNs()
and BUGs() is rather unclear. The only failure path (where an MSI-X
vector needs to be masked but the MSI-X table is not accessible) has a
useful warning message.
Signed-off-by: David Vrabel
CR: https://code.amazon.com/reviews/CR-79020927
---
xen/arch/x86/msi.c | 34 +-
1 file changed, 9 insertions(+), 25 deletions(-)
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 8bde6b9be1..6c675d11d1 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -314,7 +314,7 @@ int msi_maskable_irq(const struct msi_desc *entry)
|| entry->msi_attrib.maskbit;
}
-static bool msi_set_mask_bit(struct irq_desc *desc, bool host, bool guest)
+static void msi_set_mask_bit(struct irq_desc *desc, bool host, bool guest)
{
struct msi_desc *entry = desc->msi_desc;
struct pci_dev *pdev;
@@ -361,11 +361,6 @@ static bool msi_set_mask_bit(struct irq_desc *desc, bool
host, bool guest)
if ( likely(control & PCI_MSIX_FLAGS_ENABLE) )
break;
-
-entry->msi_attrib.host_masked = host;
-entry->msi_attrib.guest_masked = guest;
-
-flag = true;
}
else if ( flag && !(control & PCI_MSIX_FLAGS_MASKALL) )
{
@@ -385,14 +380,10 @@ static bool msi_set_mask_bit(struct irq_desc *desc, bool
host, bool guest)
control |= PCI_MSIX_FLAGS_MASKALL;
pci_conf_write16(pdev->sbdf,
msix_control_reg(entry->msi_attrib.pos), control);
-return flag;
-default:
-return 0;
+break;
}
entry->msi_attrib.host_masked = host;
entry->msi_attrib.guest_masked = guest;
-
-return 1;
}
static int msi_get_mask_bit(const struct msi_desc *entry)
@@ -418,16 +409,12 @@ static int msi_get_mask_bit(const struct msi_desc *entry)
void cf_check mask_msi_irq(struct irq_desc *desc)
{
-if ( unlikely(!msi_set_mask_bit(desc, 1,
-desc->msi_desc->msi_attrib.guest_masked)) )
-BUG_ON(!(desc->status & IRQ_DISABLED));
+msi_set_mask_bit(desc, 1, desc->msi_desc->msi_attrib.guest_masked);
}
void cf_check unmask_msi_irq(struct irq_desc *desc)
{
-if ( unlikely(!msi_set_mask_bit(desc, 0,
-desc->msi_desc->msi_attrib.guest_masked)) )
-WARN();
+msi_set_mask_bit(desc, 0, desc->msi_desc->msi_attrib.guest_masked);
}
void guest_mask_msi_irq(struct irq_desc *desc, bool mask)
@@ -437,15 +424,13 @@ void guest_mask_msi_irq(struct irq_desc *desc, bool mask)
static unsigned int cf_check startup_msi_irq(struct irq_desc *desc)
{
-if ( unlikely(!msi_set_mask_bit(desc, 0, !!(desc->status & IRQ_GUEST))) )
-WARN();
+msi_set_mask_bit(desc, 0, !!(desc->status & IRQ_GUEST));
return 0;
}
static void cf_check shutdown_msi_irq(struct irq_desc *desc)
{
-if ( unlikely(!msi_set_mask_bit(desc, 1, 1)) )
-BUG_ON(!(desc->status & IRQ_DISABLED));
+msi_set_mask_bit(desc, 1, 1);
}
void cf_check ack_nonmaskable_msi_irq(struct irq_desc *desc)
@@ -1358,10 +1343,9 @@ int pci_restore_msi_state(struct pci_dev *pdev)
for ( i = 0; ; )
{
-if ( unlikely(!msi_set_mask_bit(desc,
-entry[i].msi_attrib.host_masked,
-entry[i].msi_attrib.guest_masked))
)
-BUG();
+msi_set_mask_bit(desc,
+ entry[i].msi_attrib.host_masked,
+ entry[i].msi_attrib.guest_masked);
if ( !--nr )
break;
--
2.30.2
Re: [PATCH v3 6/9] xen/common: add cache coloring allocator for domains
On 22.10.2022 17:51, Carlo Nonato wrote:
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -661,7 +661,12 @@ static int p2m_create_table(struct p2m_domain *p2m,
> lpae_t *entry)
>
> ASSERT(!p2m_is_valid(*entry));
>
> -page = alloc_domheap_page(NULL, 0);
> +/* If cache coloring is enabled, p2m tables are allocated using the
> domain
> + * coloring configuration to prevent cache interference. */
> +if ( IS_ENABLED(CONFIG_CACHE_COLORING) )
> +page = alloc_domheap_page(p2m->domain, MEMF_no_refcount);
Are you sure you don't mean MEMF_no_owner (which implies MEMF_no_refcount)
here? And then ...
> +else
> +page = alloc_domheap_page(NULL, 0);
... is it really necessary to keep the two cases separate?
Also nit: Comment style.
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -150,6 +150,9 @@
> #define p2m_pod_offline_or_broken_hit(pg) 0
> #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
> #endif
> +#ifdef CONFIG_HAS_CACHE_COLORING
> +#include
> +#endif
>
> #ifndef PGC_static
> #define PGC_static 0
> @@ -231,6 +234,14 @@ static bool __read_mostly scrub_debug;
> #define scrub_debugfalse
> #endif
>
> +/* Memory required for buddy allocator to work with colored one */
> +#ifdef CONFIG_BUDDY_ALLOCATOR_SIZE
> +static unsigned long __initdata buddy_alloc_size =
> +CONFIG_BUDDY_ALLOCATOR_SIZE << 20;
> +#else
> +static unsigned long __initdata buddy_alloc_size = 0;
Nit: Bogus indentation. I wonder anyway whether if wouldn't better
be
static unsigned long __initdata buddy_alloc_size =
#ifdef CONFIG_BUDDY_ALLOCATOR_SIZE
CONFIG_BUDDY_ALLOCATOR_SIZE << 20;
#else
0;
#endif
or
static unsigned long __initdata buddy_alloc_size
#ifdef CONFIG_BUDDY_ALLOCATOR_SIZE
= CONFIG_BUDDY_ALLOCATOR_SIZE << 20
#endif
;
> +static void free_color_heap_page(struct page_info *pg)
> +{
> +struct page_info *pos;
> +unsigned int color = page_to_color(pg);
> +colored_pages_t *head = color_heap(color);
> +
> +spin_lock(&heap_lock);
> +
> +pg->count_info = PGC_state_free | PGC_colored;
> +page_set_owner(pg, NULL);
> +free_colored_pages[color]++;
> +
> +page_list_for_each( pos, head )
> +{
> +if ( page_to_maddr(pos) < page_to_maddr(pg) )
> +break;
> +}
I continue to view such loops as problematic. With them in place I don't
think this feature can move to being (security) supported, so I think this
and similar places want annotating with a FIXME or alike comment.
> +page_list_add_next(pg, pos, head);
>
> +spin_unlock(&heap_lock);
> +}
> +
> +static struct page_info *alloc_color_heap_page(unsigned int memflags,
> + const unsigned int *colors,
> + unsigned int num_colors)
> +{
> +struct page_info *pg = NULL;
> +unsigned int i, color;
> +bool need_tlbflush = false;
> +uint32_t tlbflush_timestamp = 0;
> +
> +spin_lock(&heap_lock);
> +
> +for ( i = 0; i < num_colors; i++ )
> +{
> +struct page_info *tmp;
> +
> +if ( page_list_empty(color_heap(colors[i])) )
> +continue;
> +
> +tmp = page_list_first(color_heap(colors[i]));
> +if ( !pg || page_to_maddr(tmp) > page_to_maddr(pg) )
> +pg = tmp;
> +}
> +
> +if ( !pg )
> +{
> +spin_unlock(&heap_lock);
> +return NULL;
> +}
> +
> +pg->count_info = PGC_state_inuse | PGC_colored;
> +
> +if ( !(memflags & MEMF_no_tlbflush) )
> +accumulate_tlbflush(&need_tlbflush, pg, &tlbflush_timestamp);
> +
> +init_free_page_fields(pg);
> +flush_page_to_ram(mfn_x(page_to_mfn(pg)),
> + !(memflags & MEMF_no_icache_flush));
> +
> +color = page_to_color(pg);
You don't really need to retrieve the color here, do you? You could as
well latch it in the loop above.
> +static void dump_color_heap(void)
> +{
> +unsigned int color;
> +
> +printk("Dumping coloring heap info\n");
> +for ( color = 0; color < get_max_colors(); color++ )
> +printk("Color heap[%u]: %lu pages\n", color,
> free_colored_pages[color]);
> +}
> +
> +integer_param("buddy-alloc-size", buddy_alloc_size);
This would preferably live next to the variable it controls, e.g. (taking
the earlier comment into account)
static unsigned long __initdata buddy_alloc_size =
#ifdef CONFIG_CACHE_COLORING
CONFIG_BUDDY_ALLOCATOR_SIZE << 20;
integer_param("buddy-alloc-size", buddy_alloc_size);
#else
0;
#endif
(Assuming buddy_alloc_size is indeed used anywhere outside any #ifdef
CONFIG_CACHE_COLORING in the first place.)
> @@ -1926,24 +2106,49 @@ static unsigned long avail_heap_pages(
> void __init end_boot_allocator(void)
> {
> unsigned int i;
> +unsigned long buddy_pages;
>
> -/* Pages that are free now go to the domain sub-allocator. */
> -for ( i = 0; i < nr_bootmem_regions; i++ )
> +
[PATCH v3] platform/x86: don't unconditionally attach Intel PMC when virtualized
The current logic in the Intel PMC driver will forcefully attach it
when detecting any CPU on the intel_pmc_core_platform_ids array,
even if the matching ACPI device is not present.
There's no checking in pmc_core_probe() to assert that the PMC device
is present, and hence on virtualized environments the PMC device
probes successfully, even if the underlying registers are not present.
Previous to 21ae43570940 the driver would check for the presence of a
specific PCI device, and that prevented the driver from attaching when
running virtualized.
Fix by only forcefully attaching the PMC device when not running
virtualized. Note that virtualized platforms can still get the device
to load if the appropriate ACPI device is present on the tables
provided to the VM.
Make an exception for the Xen initial domain, which does have full
hardware access, and hence can attach to the PMC if present.
Fixes: 21ae43570940 ('platform/x86: intel_pmc_core: Substitute PCI with CPUID
enumeration')
Signed-off-by: Roger Pau Monné
Acked-by: David E. Box
---
Changes since v2:
- Don't split condition line.
Changes since v1:
- Use cpu_feature_enabled() instead of boot_cpu_has().
---
drivers/platform/x86/intel/pmc/pltdrv.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/platform/x86/intel/pmc/pltdrv.c
b/drivers/platform/x86/intel/pmc/pltdrv.c
index 15ca8afdd973..ddfba38c2104 100644
--- a/drivers/platform/x86/intel/pmc/pltdrv.c
+++ b/drivers/platform/x86/intel/pmc/pltdrv.c
@@ -18,6 +18,8 @@
#include
#include
+#include
+
static void intel_pmc_core_release(struct device *dev)
{
kfree(dev);
@@ -53,6 +55,13 @@ static int __init pmc_core_platform_init(void)
if (acpi_dev_present("INT33A1", NULL, -1))
return -ENODEV;
+ /*
+* Skip forcefully attaching the device for VMs. Make an exception for
+* Xen dom0, which does have full hardware access.
+*/
+ if (cpu_feature_enabled(X86_FEATURE_HYPERVISOR) &&
!xen_initial_domain())
+ return -ENODEV;
+
if (!x86_match_cpu(intel_pmc_core_platform_ids))
return -ENODEV;
--
2.37.3
Re: [PATCH v2 1/2] x86: Check return values from early_memremap calls
On 11/10/22 07:45, Ross Philipson wrote:
> dt = early_memremap(initial_dtb, map_len);
> + if (!dt) {
> + pr_warn("failed to memremap initial dtb\n");
> + return;
> + }
Are all of these new pr_warn/err()'s really adding much value? They all
look pretty generic. It makes me wonder if we should just spit out a
generic message in early_memremap() and save all the callers the trouble.
Oh, and don't we try to refer to functions() with parenthesis?
Re: [PATCH v2 1/2] x86: Check return values from early_memremap calls
On 10.11.22 16:45, Ross Philipson wrote: There are a number of places where early_memremap is called but the return pointer is not checked for NULL. The call can result in a NULL being returned so the checks must be added. Note that the maintainers for both the Jailhouse and Xen code approved of using panic() to handle allocation failures. In addition to checking the return values, a bit of extra cleanup of pr_* usages was done since the pr_fmt macro was introduced in the modules touched by this patch. Signed-off-by: Ross Philipson --- arch/x86/kernel/devicetree.c | 13 +++ arch/x86/kernel/e820.c | 12 +-- arch/x86/kernel/jailhouse.c | 6 ++ arch/x86/kernel/mpparse.c| 51 +--- arch/x86/kernel/setup.c | 19 ++--- arch/x86/xen/enlighten_hvm.c | 2 ++ arch/x86/xen/mmu_pv.c| 8 +++ arch/x86/xen/setup.c | 2 ++ 8 files changed, 95 insertions(+), 18 deletions(-) For the Xen parts: Reviewed-by: Juergen Gross Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
[PATCH v2 0/2] x86: Check return values for early memory/IO remap calls
While sending an earlier patch set it was discovered that there are a number of places in early x86 code were the functions early_memremap() and early_ioremap() are called but the returned pointer is not checked for NULL. Since NULL can be returned for a couple of reasons, the return value should be checked for NULL. This set fixes the places where the checks were missing. It was not always clear what the best failure mode should be when NULL is detected. In modules where other places tended to pr_warn or panic e.g., the same was done for the checks. In other places it was based on how significantly fatal the failure would end up being. The review process may point out places where this should be changed. Changes in v2: - Added notes in comments about why panic() was used in some cases and the fact that maintainers approved the usage. - Added pr_fmt macros in changed files to allow proper usage of pr_* printing macros. Ross Philipson (2): x86: Check return values from early_memremap calls x86: Check return values from early_ioremap calls arch/x86/kernel/apic/x2apic_uv_x.c | 2 ++ arch/x86/kernel/devicetree.c | 13 ++ arch/x86/kernel/e820.c | 12 +++-- arch/x86/kernel/early_printk.c | 2 ++ arch/x86/kernel/jailhouse.c| 6 + arch/x86/kernel/mpparse.c | 51 -- arch/x86/kernel/setup.c| 19 +++--- arch/x86/kernel/vsmp_64.c | 3 +++ arch/x86/xen/enlighten_hvm.c | 2 ++ arch/x86/xen/mmu_pv.c | 8 ++ arch/x86/xen/setup.c | 2 ++ 11 files changed, 102 insertions(+), 18 deletions(-) -- 1.8.3.1
Re: [PATCH] xen/pcpu: fix possible memory leak in register_pcpu()
On 10.11.22 16:24, Yang Yingliang wrote:
In device_add(), dev_set_name() is called to allocate name, if it returns
error, the name need be freed. As comment of device_register() says, it
should use put_device() to give up the reference in the error path. So fix
this by calling put_device(), then the name can be freed in kobject_cleanup().
Fixes: f65c9bb3fb72 ("xen/pcpu: Xen physical cpus online/offline sys interface")
Signed-off-by: Yang Yingliang
Reviewed-by: Juergen Gross
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
[PATCH v2 2/2] x86: Check return values from early_ioremap calls
There are a number of places where early_ioremap is called
but the return pointer is not checked for NULL. The call
can result in a NULL being returned so the checks must
be added.
On allocation failures, panic() was used since this seemed
to be the action taken on other failures in the modules
touched by this patch.
Signed-off-by: Ross Philipson
---
arch/x86/kernel/apic/x2apic_uv_x.c | 2 ++
arch/x86/kernel/early_printk.c | 2 ++
arch/x86/kernel/vsmp_64.c | 3 +++
3 files changed, 7 insertions(+)
diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c
b/arch/x86/kernel/apic/x2apic_uv_x.c
index 4828552..4ffdc27 100644
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -75,6 +75,8 @@ static unsigned long __init uv_early_read_mmr(unsigned long
addr)
unsigned long val, *mmr;
mmr = early_ioremap(UV_LOCAL_MMR_BASE | addr, sizeof(*mmr));
+ if (!mmr)
+ panic("UV: error: failed to ioremap MMR\n");
val = *mmr;
early_iounmap(mmr, sizeof(*mmr));
diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
index 44f9370..1fe590d 100644
--- a/arch/x86/kernel/early_printk.c
+++ b/arch/x86/kernel/early_printk.c
@@ -290,6 +290,8 @@ static __init void early_pci_serial_init(char *s)
/* WARNING! assuming the address is always in the first 4G */
early_serial_base =
(unsigned long)early_ioremap(bar0 &
PCI_BASE_ADDRESS_MEM_MASK, 0x10);
+ if (!early_serial_base)
+ panic("early_serial: failed to ioremap MMIO BAR\n");
write_pci_config(bus, slot, func, PCI_COMMAND,
cmdreg|PCI_COMMAND_MEMORY);
}
diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index 796cfaa..39769f4 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -32,6 +32,9 @@ static void __init set_vsmp_ctl(void)
/* set vSMP magic bits to indicate vSMP capable kernel */
cfg = read_pci_config(0, 0x1f, 0, PCI_BASE_ADDRESS_0);
address = early_ioremap(cfg, 8);
+ if (WARN_ON(!address))
+ return;
+
cap = readl(address);
ctl = readl(address + 4);
printk(KERN_INFO "vSMP CTL: capabilities:0x%08x control:0x%08x\n",
--
1.8.3.1
[PATCH v2 1/2] x86: Check return values from early_memremap calls
There are a number of places where early_memremap is called
but the return pointer is not checked for NULL. The call
can result in a NULL being returned so the checks must
be added.
Note that the maintainers for both the Jailhouse and Xen code
approved of using panic() to handle allocation failures.
In addition to checking the return values, a bit of extra
cleanup of pr_* usages was done since the pr_fmt macro was
introduced in the modules touched by this patch.
Signed-off-by: Ross Philipson
---
arch/x86/kernel/devicetree.c | 13 +++
arch/x86/kernel/e820.c | 12 +--
arch/x86/kernel/jailhouse.c | 6 ++
arch/x86/kernel/mpparse.c| 51 +---
arch/x86/kernel/setup.c | 19 ++---
arch/x86/xen/enlighten_hvm.c | 2 ++
arch/x86/xen/mmu_pv.c| 8 +++
arch/x86/xen/setup.c | 2 ++
8 files changed, 95 insertions(+), 18 deletions(-)
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 5cd51f2..4a5ca9a 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -2,6 +2,9 @@
/*
* Architecture specific OF callbacks.
*/
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include
#include
#include
@@ -292,10 +295,20 @@ static void __init x86_flattree_get_config(void)
map_len = max(PAGE_SIZE - (initial_dtb & ~PAGE_MASK), (u64)128);
dt = early_memremap(initial_dtb, map_len);
+ if (!dt) {
+ pr_warn("failed to memremap initial dtb\n");
+ return;
+ }
+
size = fdt_totalsize(dt);
if (map_len < size) {
early_memunmap(dt, map_len);
dt = early_memremap(initial_dtb, size);
+ if (!dt) {
+ pr_warn("failed to memremap initial dtb\n");
+ return;
+ }
+
map_len = size;
}
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 9dac246..9cbc724 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -9,6 +9,9 @@
* quirks and other tweaks, and feeds that into the generic Linux memory
* allocation code routines via a platform independent interface (memblock,
etc.).
*/
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include
#include
#include
@@ -728,6 +731,11 @@ void __init e820__memory_setup_extended(u64 phys_addr, u32
data_len)
struct setup_data *sdata;
sdata = early_memremap(phys_addr, data_len);
+ if (!sdata) {
+ pr_warn("failed to memremap extended\n");
+ return;
+ }
+
entries = sdata->len / sizeof(*extmap);
extmap = (struct boot_e820_entry *)(sdata->data);
@@ -1007,7 +1015,7 @@ void __init e820__reserve_setup_data(void)
while (pa_data) {
data = early_memremap(pa_data, sizeof(*data));
if (!data) {
- pr_warn("e820: failed to memremap setup_data entry\n");
+ pr_warn("failed to memremap setup_data entry\n");
return;
}
@@ -1030,7 +1038,7 @@ void __init e820__reserve_setup_data(void)
early_memunmap(data, sizeof(*data));
data = early_memremap(pa_data, len);
if (!data) {
- pr_warn("e820: failed to memremap indirect
setup_data\n");
+ pr_warn("failed to memremap indirect
setup_data\n");
return;
}
diff --git a/arch/x86/kernel/jailhouse.c b/arch/x86/kernel/jailhouse.c
index 4eb8f2d..80db0c2 100644
--- a/arch/x86/kernel/jailhouse.c
+++ b/arch/x86/kernel/jailhouse.c
@@ -221,6 +221,9 @@ static void __init jailhouse_init_platform(void)
while (pa_data) {
mapping = early_memremap(pa_data, sizeof(header));
+ if (!mapping)
+ panic("Jailhouse: failed to memremap setup_data
header\n");
+
memcpy(&header, mapping, sizeof(header));
early_memunmap(mapping, sizeof(header));
@@ -241,6 +244,9 @@ static void __init jailhouse_init_platform(void)
setup_data_len = min_t(unsigned long, sizeof(setup_data),
(unsigned long)header.len);
mapping = early_memremap(pa_data, setup_data_len);
+ if (!mapping)
+ panic("Jailhouse: failed to memremap setup_data\n");
+
memcpy(&setup_data, mapping, setup_data_len);
early_memunmap(mapping, setup_data_len);
diff --git a/arch/x86/kernel/mpparse.c b/arch/x86/kernel/mpparse.c
index fed721f..4254163 100644
--- a/arch/x86/kernel/mpparse.c
+++ b/arch/x86/kernel/mpparse.c
@@ -8,6 +8,8 @@
* (c) 2008 Alexey Starikovskiy
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include
#include
#include
@@ -145,33 +147,33 @@ static int __init smp_check_mpc(struct mpc_tab
Re: [PATCH v3 0/4] Yocto Gitlab CI
Hi Anthony, > On 10 Nov 2022, at 13:20, Anthony PERARD wrote: > > On Mon, Nov 07, 2022 at 08:50:09AM +0100, Michal Orzel wrote: >> 3) Try to use CI to build and push the containers to registry >> - it could be possible but what about local users > > FYI, it's already possible to build and push container from the CI, at > least on X86, I've made it work: > > https://lore.kernel.org/all/20220301121133.19271-3-anthony.per...@citrix.com/ > This works only when pushing to the "staging" branch on the main "xen" > repo as I've added secret variable there. (Also the branch needs to be > "protected" if I remember correctly.) Very nice :-) Would definitely be a good solution to have a solution like that for yocto. > > I don't know how to use the Arm runner to do container builds like that. > > (The change done to one x86 runner: > https://gitlab.com/xen-project/xen-gitlab-ci/-/merge_requests/15) Something to look at definitely. Cheers Bertrand > > Cheers, > > -- > Anthony PERARD
Re: [PATCH] xen/notifier: simplify using notifier_[to|from]_errno()
On 28.10.2022 13:41, Juergen Gross wrote: > Today all users of notifier_from_errno() and notifier_to_errno() are > Handling the success case the same way, by using > > !rc ? NOTIFY_DONE : notifier_from_errno(rc) > > or > > (notifier_rc == NOTIFY_DONE) ? 0 : notifier_to_errno(notifier_rc); > > Simplify the use cases by moving the handling of the success case into > the functions. > > Signed-off-by: Juergen Gross Acked-by: Jan Beulich
[PATCH] xen/pcpu: fix possible memory leak in register_pcpu()
In device_add(), dev_set_name() is called to allocate name, if it returns
error, the name need be freed. As comment of device_register() says, it
should use put_device() to give up the reference in the error path. So fix
this by calling put_device(), then the name can be freed in kobject_cleanup().
Fixes: f65c9bb3fb72 ("xen/pcpu: Xen physical cpus online/offline sys interface")
Signed-off-by: Yang Yingliang
---
drivers/xen/pcpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c
index 47aa3a1ccaf5..fd3a644b0855 100644
--- a/drivers/xen/pcpu.c
+++ b/drivers/xen/pcpu.c
@@ -228,7 +228,7 @@ static int register_pcpu(struct pcpu *pcpu)
err = device_register(dev);
if (err) {
- pcpu_release(dev);
+ put_device(dev);
return err;
}
--
2.25.1
Xen Security Advisory 422 v2 (CVE-2022-23824) - x86: Multiple speculative security issues
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Xen Security Advisory CVE-2022-23824 / XSA-422 version 2 x86: Multiple speculative security issues UPDATES IN VERSION 2 Change the URL referenced for the Branch Type Confusion update. ISSUE DESCRIPTION = 1) Researchers have discovered that on some AMD CPUs, the implementation of IBPB (Indirect Branch Prediction Barrier) does not behave according to the specification. Specifically, IBPB fails to properly flush the RAS (Return Address Stack, also RSB - Return Stack Buffer - in Intel terminology; one of the hardware prediction structures), allowing attacker controlled values to survive across a deliberate attempt to purge said values. AMD have allocated CVE-2022-23824. For more details, see: https://www.amd.com/en/corporate/product-security/bulletin/amd-sb-1040 2) AMD have discovered that under some circumstances, the previous reported information about Branch Type Confusion (XSA-407 / CVE-2022-23825) was inaccurate. Specifically, it was previously reported that the small speculation window was not long enough to contain two dependent loads. It has turned out not to be true, and in some circumstances, the speculation window is long enough to contain two dependent loads. AMD have not allocated a new CVE for this issue. For more details, see: https://www.amd.com/system/files/documents/technical-guidance-for-mitigating-branch-type-confusion.pdf IMPACT == An attacker might be able to infer the contents of memory belonging to other guests. Due to the interaction of this issue with previous speculation fixes in their default configuration, an attacker cannot leverage this vulnerability to infer the content of memory that belongs to Xen itself. VULNERABLE SYSTEMS == Systems running all versions of Xen are affected. Only AMD CPUs are potentially vulnerable. CPUs from other hardware vendors are not impacted. Whether a CPU is potentially vulnerable depends on its microarchitecture. Consult your hardware vendor. The fix for XSA-407 / CVE-2022-23825 elected, out of an abundance of caution, to use IBPB-on-entry as a Branch Type Confusion mitigation. It is believed that this mitigation is still sufficient, in light of the new discoveries. Therefore, no changes are being provided at this time. For CVE-2022-23824, patches are being provided on all releases as the bug pertains to a specific speculation control not working as documented, but there are a number circumstances where safety is provided as a side effect of other speculative mitigations. * The issue is that IBPB doesn't flush the RAS (Return Address Stack). Also called the RSB (Return Stack Buffer) in Intel terminology. Xen tends to follow Intel's terminology. * By default, Xen uses IBPB on a context switch from one vCPU to another vCPU to prevent guest to guest attacks. This action is not about protecting Xen from a malicious guest; such protections are elsewhere. * By default, Xen flushes the RAS/RSB on VMExit from HVM/PVH vCPUs, in order to protect itself from a malicious vCPU. Therefore, a malicious HVM/PVH guest cannot mount an attack using this vulnerability. * Whether Xen flushes the RAS/RSB by default on exit from PV vCPUs (again, to protect itself) is more complicated. There is an optimisation commonly used by native OSes when the SMEP (Supervisor Mode Execution Prevention) feature is active, which Xen can make use in some cases. - Xen 4.15 and older flush the RAS/RSB by default. - Xen 4.16 introduced an optimisation to skip flushing the RAS/RSB when safe. For CPUs impacted by CVE-2022-23824, this comes down to whether 32-bit PV guest support is enabled or not; *irrespective* of whether any 32-bit PV guests are actively running. If Xen is built with CONFIG_PV32=n, or Xen is booted with `pv=no-32`, or 32-bit PV guests are disabled as a side effect of CET being active (requires a capable toolchain, CONFIG_XEN_SHSTK=y or CONFIG_XEN_IBT=y, and capable hardware), then Xen will by default use the performance optimisation. In this case, a malicious 64-bit PV guest can mount an attack using this issue. Note: This analysis is only applicable for systems which are fully up to date with previous speculation-related XSAs, and have not used `spec-ctrl=` on the Xen command line to tune the speculative mitigations. MITIGATION == If there are untrusted 64-bit PV guests on the system on a Xen 4.16 or later system, specifying `spec-ctrl=rsb` on Xen's command line and rebooting will mitigate the vulnerability. RESOLUTION == Applying the appropriate set of patches resolves this issue. Note that patches for released versions are generally prepared to apply to the stable branches, and may not apply cleanly to
Re: [PATCH v2] platform/x86: don't unconditionally attach Intel PMC when virtualized
On Thu, Nov 10, 2022 at 02:33:35PM +0100, Roger Pau Monne wrote:
> The current logic in the Intel PMC driver will forcefully attach it
> when detecting any CPU on the intel_pmc_core_platform_ids array,
> even if the matching ACPI device is not present.
>
> There's no checking in pmc_core_probe() to assert that the PMC device
> is present, and hence on virtualized environments the PMC device
> probes successfully, even if the underlying registers are not present.
> Previous to 21ae43570940 the driver would check for the presence of a
> specific PCI device, and that prevented the driver from attaching when
> running virtualized.
>
> Fix by only forcefully attaching the PMC device when not running
> virtualized. Note that virtualized platforms can still get the device
> to load if the appropriate ACPI device is present on the tables
> provided to the VM.
>
> Make an exception for the Xen initial domain, which does have full
> hardware access, and hence can attach to the PMC if present.
>
> Fixes: 21ae43570940 ('platform/x86: intel_pmc_core: Substitute PCI with CPUID
> enumeration')
> Signed-off-by: Roger Pau Monné
> Acked-by: David E. Box
> Cc: Rajneesh Bhardwaj
> Cc: David E Box
> Cc: Hans de Goede
> Cc: Mark Gross
> Cc: Andy Shevchenko
> Cc: Srinivas Pandruvada
> Cc: Juergen Gross
> Cc: platform-driver-...@vger.kernel.org
> Cc: xen-devel@lists.xenproject.org
You may use --cc to the sending tool, instead of polluting a commit message
with that. Moreover, the Cc list will be archived on lore.kernel.org anyway,
in case you really need it to be recorded.
...
> + if (cpu_feature_enabled(X86_FEATURE_HYPERVISOR) &&
> + !xen_initial_domain())
One line? It's 81 character only and we have no strong 80 here, IIRC.
> + return -ENODEV;
--
With Best Regards,
Andy Shevchenko
[PATCH v2] platform/x86: don't unconditionally attach Intel PMC when virtualized
The current logic in the Intel PMC driver will forcefully attach it
when detecting any CPU on the intel_pmc_core_platform_ids array,
even if the matching ACPI device is not present.
There's no checking in pmc_core_probe() to assert that the PMC device
is present, and hence on virtualized environments the PMC device
probes successfully, even if the underlying registers are not present.
Previous to 21ae43570940 the driver would check for the presence of a
specific PCI device, and that prevented the driver from attaching when
running virtualized.
Fix by only forcefully attaching the PMC device when not running
virtualized. Note that virtualized platforms can still get the device
to load if the appropriate ACPI device is present on the tables
provided to the VM.
Make an exception for the Xen initial domain, which does have full
hardware access, and hence can attach to the PMC if present.
Fixes: 21ae43570940 ('platform/x86: intel_pmc_core: Substitute PCI with CPUID
enumeration')
Signed-off-by: Roger Pau Monné
Acked-by: David E. Box
Cc: Rajneesh Bhardwaj
Cc: David E Box
Cc: Hans de Goede
Cc: Mark Gross
Cc: Andy Shevchenko
Cc: Srinivas Pandruvada
Cc: Juergen Gross
Cc: platform-driver-...@vger.kernel.org
Cc: xen-devel@lists.xenproject.org
---
Changes since v1:
- Use cpu_feature_enabled() instead of boot_cpu_has().
---
drivers/platform/x86/intel/pmc/pltdrv.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/platform/x86/intel/pmc/pltdrv.c
b/drivers/platform/x86/intel/pmc/pltdrv.c
index 15ca8afdd973..4f9fe8a21d8f 100644
--- a/drivers/platform/x86/intel/pmc/pltdrv.c
+++ b/drivers/platform/x86/intel/pmc/pltdrv.c
@@ -18,6 +18,8 @@
#include
#include
+#include
+
static void intel_pmc_core_release(struct device *dev)
{
kfree(dev);
@@ -53,6 +55,14 @@ static int __init pmc_core_platform_init(void)
if (acpi_dev_present("INT33A1", NULL, -1))
return -ENODEV;
+ /*
+* Skip forcefully attaching the device for VMs. Make an exception for
+* Xen dom0, which does have full hardware access.
+*/
+ if (cpu_feature_enabled(X86_FEATURE_HYPERVISOR) &&
+ !xen_initial_domain())
+ return -ENODEV;
+
if (!x86_match_cpu(intel_pmc_core_platform_ids))
return -ENODEV;
--
2.37.3
Re: [PATCH v3 0/4] Yocto Gitlab CI
On Mon, Nov 07, 2022 at 08:50:09AM +0100, Michal Orzel wrote: > 3) Try to use CI to build and push the containers to registry > - it could be possible but what about local users FYI, it's already possible to build and push container from the CI, at least on X86, I've made it work: https://lore.kernel.org/all/20220301121133.19271-3-anthony.per...@citrix.com/ This works only when pushing to the "staging" branch on the main "xen" repo as I've added secret variable there. (Also the branch needs to be "protected" if I remember correctly.) I don't know how to use the Arm runner to do container builds like that. (The change done to one x86 runner: https://gitlab.com/xen-project/xen-gitlab-ci/-/merge_requests/15) Cheers, -- Anthony PERARD
[xen-4.16-testing test] 174695: tolerable FAIL - PUSHED
flight 174695 xen-4.16-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/174695/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 174678 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 174678 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 174678 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 174678 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 174678 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 174678 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 174678 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 174678 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 174678 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 174678 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 174678 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 174678 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-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-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-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-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass version targeted for testing: xen 1dc6dccb1a8752f200ec2612b2bd091bbf88b231 baseline version: xen c1e196ab490b
[xen-4.15-testing test] 174690: tolerable FAIL - PUSHED
flight 174690 xen-4.15-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/174690/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 174572 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 174572 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 174572 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 174572 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 174572 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 174572 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 174572 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 174572 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 174572 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 174572 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 174572 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 174572 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-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-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass version targeted for testing: xen 625efe28ab5309ab83f7826ed1de4966ede2f191 baseline version: xen e818f4f0dabf
Re: [PATCH v3 0/4] Yocto Gitlab CI
Hello, On 10/11/2022 11:08, Bertrand Marquis wrote: > > > Hi Michal, > >> On 10 Nov 2022, at 07:34, Michal Orzel wrote: >> >> Hi Stefano, >> >> On 10/11/2022 01:18, Stefano Stabellini wrote: >>> >>> >>> On Mon, 7 Nov 2022, Michal Orzel wrote: Hi Bertrand and Stefano, On 31/10/2022 16:00, Bertrand Marquis wrote: > > > Hi Michal, > >> On 31 Oct 2022, at 14:39, Michal Orzel wrote: >> >> Hi Bertrand, >> >> On 31/10/2022 15:00, Bertrand Marquis wrote: >>> >>> >>> This patch series is a first attempt to check if we could use Yocto in >>> gitlab ci to build and run xen on qemu for arm, arm64 and x86. >>> >>> The first patch is creating a container with all elements required to >>> build Yocto, a checkout of the yocto layers required and an helper >>> script to build and run xen on qemu with yocto. >>> >>> The second patch is creating containers with a first build of yocto done >>> so that susbsequent build with those containers would only rebuild what >>> was changed and take the rest from the cache. >>> >>> The third patch is adding a way to easily clean locally created >>> containers. >>> >>> This is is mainly for discussion and sharing as there are still some >>> issues/problem to solve: >>> - building the qemu* containers can take several hours depending on the >>> network bandwith and computing power of the machine where those are >>> created >> This is not really an issue as the build of the containers occurs on the >> local >> machines before pushing them to registry. Also, building the containers >> will only be required for new Yocto releases. >> >>> - produced containers containing the cache have a size between 8 and >>> 12GB depending on the architecture. We might need to store the build >>> cache somewhere else to reduce the size. If we choose to have one >>> single image, the needed size is around 20GB and we need up to 40GB >>> during the build, which is why I splitted them. >>> - during the build and run, we use a bit more then 20GB of disk which is >>> over the allowed size in gitlab >> As we could see during v2 testing, we do not have any space restrictions >> on the Xen GitLab and I think we already decided to have the Yocto >> integrated into our CI. > > Right, I should have modified this chapter to be coherent with your > latest tests. > Sorry for that. > >> >> I will do some testing and get back to you with results + review. I did some testing and here are the results: In the current form this series will fail when running CI because the Yocto containers are based on "From ubuntu:22.04" (there is no platform prefix), which means that the containers are built for the host architecture (in my case and in 99% of the cases of the local build it will be x86). In Gitlab we have 2 runners (arm64 and x86_64). This means that all the test jobs would need to specify x86_64 as a tag when keeping the current behavior. After I built all the containers on my x86 machine, I pushed them to registry and the pipeline was successful: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com%2Fxen-project%2Fpeople%2Fmorzel%2Fxen-orzelmichal%2F-%2Fpipelines%2F686853939&data=05%7C01%7Cmichal.orzel%40amd.com%7Ccc0a420856c64224e78208dac3037b95%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638036716985754450%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ARpspaYQlf7gujC11cS4dnR275cCJ6tMRC2J0FyhFIM%3D&reserved=0 >>> >>> When I tested the previous version of this series I built the >>> containers natively on ARM64, so that is also an option. >>> >>> Here is the diff on patch no. 3 to make the series work (using x86 tag and small improvement to include needs: []): ``` diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml index 5c620fefce59..52cccec6f904 100644 --- a/automation/gitlab-ci/test.yaml +++ b/automation/gitlab-ci/test.yaml @@ -65,6 +65,9 @@ paths: - 'logs/*' when: always + needs: [] + tags: +- x86_64 # Test jobs build-each-commit-gcc: @@ -206,19 +209,13 @@ yocto-qemuarm64: extends: .yocto-test variables: YOCTO_BOARD: qemuarm64 - tags: -- arm64 yocto-qemuarm: extends: .yocto-test variables: YOCTO_BOARD: qemuarm - tags: -- arm32 yocto-qemux86-64: extends: .yocto-test variables: YOCTO_BOARD: qemux86-64 - tags: -- x86_64 ``` Now, the logical way would be to build x86 yocto container for x86, arm64 for arm64 and arm32 on arm64 or x86. I t
RE: [PATCH for-4.17 v3 07/15] CODING_STYLE(tools/ocaml): add 'make format' and remove tabs
Hi Christian, > -Original Message- > From: Christian Lindig > Subject: Re: [PATCH for-4.17 v3 07/15] CODING_STYLE(tools/ocaml): add > 'make format' and remove tabs > > On 10 Nov 2022, at 09:25, Henry Wang wrote: > > > > Hi Christian, > > > >> -Original Message- > >> From: Christian Lindig > >> Subject: Re: [PATCH for-4.17 v3 07/15] CODING_STYLE(tools/ocaml): add > >> 'make format' and remove tabs > While I understand the goal and support, this seems to be a bit too late > to do it in Xen 4.17 (we are only a couple of weeks away). At this stage > of the release we should only do bug fix. > > This is clearly only a comesmetic change and there I would argue this > should be deferred to 4.18. That said the last call is from the RM. > >>> > >>> I agree with your point. I think maybe defer the patch to 4.18 is better, > >>> given the deep freeze state we are currently in. > >> > >> I disagree. This is an automated change that can be verified to not add > >> functional changes. Edvin has demonstrated that wrong indentation has > >> mislead reviewers in the past and caused bugs. Nobody except Edvin has > >> contributed to the affected code in years and thus it is not a burden on > the > >> project outside the OCaml part. I suggest to accept this. > > > > I understand points from you, Edwin and Julien, but I think in the earlier > > discussion in this thread, Julien has provided an argument [1] which I do > > think is a valid reason to defer this patch a little bit. > > > > But since you are the only maintainer of the Ocaml code, so if you strongly > > insist this patch should be included for the release and there would not be > > any more explicit objections from others in the next couple of days, I > > think I > > will provide my release-ack for the purpose of respecting opinions from the > > maintainer. Hope this solution should be acceptable to you. > > Thanks Henry. I think the argument here is the balance between maintaining > a policy against late large changes and improving the quality and the > reliability of future patches by using more automation. I agree that large > functional changes and any change that can’t be verified should be avoided > but I don’t think this case is one. However, > I am fine deferring the patch based on an agreed policy if we can make it a > priority to get in in soon. Thanks for your understanding. I will take a note of this patch and try to ping committers to commit this patch as soon as the staging tree gets unfrozen after the release. In this way I think your concerns in... > For me this is part of improving the OCaml code > base and project quality by using more automation in formatting and the > build system that lowers the barrier for contributors such that they don’t > have to worry about procedural aspects like tabs, spaces, indentation, or > build systems. ...here would be minimized. I do understand your points and frustration. Thanks again for your understanding. Kind regards, Henry > > — C
Re: [PATCH v3 0/4] Yocto Gitlab CI
Hi Michal, > On 10 Nov 2022, at 07:34, Michal Orzel wrote: > > Hi Stefano, > > On 10/11/2022 01:18, Stefano Stabellini wrote: >> >> >> On Mon, 7 Nov 2022, Michal Orzel wrote: >>> Hi Bertrand and Stefano, >>> >>> On 31/10/2022 16:00, Bertrand Marquis wrote: Hi Michal, > On 31 Oct 2022, at 14:39, Michal Orzel wrote: > > Hi Bertrand, > > On 31/10/2022 15:00, Bertrand Marquis wrote: >> >> >> This patch series is a first attempt to check if we could use Yocto in >> gitlab ci to build and run xen on qemu for arm, arm64 and x86. >> >> The first patch is creating a container with all elements required to >> build Yocto, a checkout of the yocto layers required and an helper >> script to build and run xen on qemu with yocto. >> >> The second patch is creating containers with a first build of yocto done >> so that susbsequent build with those containers would only rebuild what >> was changed and take the rest from the cache. >> >> The third patch is adding a way to easily clean locally created >> containers. >> >> This is is mainly for discussion and sharing as there are still some >> issues/problem to solve: >> - building the qemu* containers can take several hours depending on the >> network bandwith and computing power of the machine where those are >> created > This is not really an issue as the build of the containers occurs on the > local > machines before pushing them to registry. Also, building the containers > will only be required for new Yocto releases. > >> - produced containers containing the cache have a size between 8 and >> 12GB depending on the architecture. We might need to store the build >> cache somewhere else to reduce the size. If we choose to have one >> single image, the needed size is around 20GB and we need up to 40GB >> during the build, which is why I splitted them. >> - during the build and run, we use a bit more then 20GB of disk which is >> over the allowed size in gitlab > As we could see during v2 testing, we do not have any space restrictions > on the Xen GitLab and I think we already decided to have the Yocto > integrated into our CI. Right, I should have modified this chapter to be coherent with your latest tests. Sorry for that. > > I will do some testing and get back to you with results + review. >>> I did some testing and here are the results: >>> >>> In the current form this series will fail when running CI because the Yocto >>> containers >>> are based on "From ubuntu:22.04" (there is no platform prefix), which means >>> that the containers >>> are built for the host architecture (in my case and in 99% of the cases of >>> the local build it will >>> be x86). In Gitlab we have 2 runners (arm64 and x86_64). This means that >>> all the test jobs would need >>> to specify x86_64 as a tag when keeping the current behavior. >>> After I built all the containers on my x86 machine, I pushed them to >>> registry and the pipeline was successful: >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com%2Fxen-project%2Fpeople%2Fmorzel%2Fxen-orzelmichal%2F-%2Fpipelines%2F686853939&data=05%7C01%7Cmichal.orzel%40amd.com%7C2449f063e67341c3b95a08dac2b112a5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638036363027707274%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=EwTJrW2vuwQIugKc7mnzG9NNbsYLP6tw5UODzBMmPEE%3D&reserved=0 >> >> When I tested the previous version of this series I built the >> containers natively on ARM64, so that is also an option. >> >> >>> Here is the diff on patch no. 3 to make the series work (using x86 tag and >>> small improvement to include needs: []): >>> ``` >>> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml >>> index 5c620fefce59..52cccec6f904 100644 >>> --- a/automation/gitlab-ci/test.yaml >>> +++ b/automation/gitlab-ci/test.yaml >>> @@ -65,6 +65,9 @@ >>> paths: >>> - 'logs/*' >>> when: always >>> + needs: [] >>> + tags: >>> +- x86_64 >>> >>> # Test jobs >>> build-each-commit-gcc: >>> @@ -206,19 +209,13 @@ yocto-qemuarm64: >>> extends: .yocto-test >>> variables: >>> YOCTO_BOARD: qemuarm64 >>> - tags: >>> -- arm64 >>> >>> yocto-qemuarm: >>> extends: .yocto-test >>> variables: >>> YOCTO_BOARD: qemuarm >>> - tags: >>> -- arm32 >>> >>> yocto-qemux86-64: >>> extends: .yocto-test >>> variables: >>> YOCTO_BOARD: qemux86-64 >>> - tags: >>> -- x86_64 >>> ``` >>> >>> Now, the logical way would be to build x86 yocto container for x86, arm64 >>> for arm64 and arm32 on arm64 or x86. >>> I tried building the container qemuarm64 specifying target arm64 on x86. >>> After 15h, only 70% of the Yocto build >>> was completed and there was an error with glibc
Re: [PATCH for-4.17 v3 07/15] CODING_STYLE(tools/ocaml): add 'make format' and remove tabs
> On 10 Nov 2022, at 09:25, Henry Wang wrote: > > Hi Christian, > >> -Original Message- >> From: Christian Lindig >> Subject: Re: [PATCH for-4.17 v3 07/15] CODING_STYLE(tools/ocaml): add >> 'make format' and remove tabs While I understand the goal and support, this seems to be a bit too late to do it in Xen 4.17 (we are only a couple of weeks away). At this stage of the release we should only do bug fix. This is clearly only a comesmetic change and there I would argue this should be deferred to 4.18. That said the last call is from the RM. >>> >>> I agree with your point. I think maybe defer the patch to 4.18 is better, >>> given the deep freeze state we are currently in. >> >> I disagree. This is an automated change that can be verified to not add >> functional changes. Edvin has demonstrated that wrong indentation has >> mislead reviewers in the past and caused bugs. Nobody except Edvin has >> contributed to the affected code in years and thus it is not a burden on the >> project outside the OCaml part. I suggest to accept this. > > I understand points from you, Edwin and Julien, but I think in the earlier > discussion in this thread, Julien has provided an argument [1] which I do > think is a valid reason to defer this patch a little bit. > > But since you are the only maintainer of the Ocaml code, so if you strongly > insist this patch should be included for the release and there would not be > any more explicit objections from others in the next couple of days, I think I > will provide my release-ack for the purpose of respecting opinions from the > maintainer. Hope this solution should be acceptable to you. Thanks Henry. I think the argument here is the balance between maintaining a policy against late large changes and improving the quality and the reliability of future patches by using more automation. I agree that large functional changes and any change that can’t be verified should be avoided but I don’t think this case is one. However, I am fine deferring the patch based on an agreed policy if we can make it a priority to get in in soon. For me this is part of improving the OCaml code base and project quality by using more automation in formatting and the build system that lowers the barrier for contributors such that they don’t have to worry about procedural aspects like tabs, spaces, indentation, or build systems. — C
RE: [PATCH for-4.17 v3 07/15] CODING_STYLE(tools/ocaml): add 'make format' and remove tabs
Hi Christian, > -Original Message- > From: Christian Lindig > Subject: Re: [PATCH for-4.17 v3 07/15] CODING_STYLE(tools/ocaml): add > 'make format' and remove tabs > >> While I understand the goal and support, this seems to be a bit too late > >> to do it in Xen 4.17 (we are only a couple of weeks away). At this stage > >> of the release we should only do bug fix. > >> > >> This is clearly only a comesmetic change and there I would argue this > >> should be deferred to 4.18. That said the last call is from the RM. > > > > I agree with your point. I think maybe defer the patch to 4.18 is better, > > given the deep freeze state we are currently in. > > I disagree. This is an automated change that can be verified to not add > functional changes. Edvin has demonstrated that wrong indentation has > mislead reviewers in the past and caused bugs. Nobody except Edvin has > contributed to the affected code in years and thus it is not a burden on the > project outside the OCaml part. I suggest to accept this. I understand points from you, Edwin and Julien, but I think in the earlier discussion in this thread, Julien has provided an argument [1] which I do think is a valid reason to defer this patch a little bit. But since you are the only maintainer of the Ocaml code, so if you strongly insist this patch should be included for the release and there would not be any more explicit objections from others in the next couple of days, I think I will provide my release-ack for the purpose of respecting opinions from the maintainer. Hope this solution should be acceptable to you. [1] https://lore.kernel.org/xen-devel/1f8c90cd-8037-84eb-d6f7-c639f8a87...@xen.org/ Kind regards, Henry > > — C > > >
Re: [PATCH for-4.17?] x86/paging: return -EINVAL for paging domctls for dying domains
On 09.11.2022 14:22, Roger Pau Monné wrote:
> On Wed, Nov 09, 2022 at 01:02:28PM +0100, Jan Beulich wrote:
>> On 09.11.2022 12:36, Roger Pau Monné wrote:
>>> Since I don't see replies to my other comments, do you agree on
>>> returning an error then?
>>
>> No, my view there hasn't changed. I wouldn't block a change to go in
>> early for 4.18, but I also wouldn't ack such.
>>
>> Perhaps just one remark on your other earlier comments: While you're
>> right about XEN_DOMCTL_SHADOW_OP_{CLEAN,PEEK}, (effectively) random
>> data in the bitmap may cause a caller to do extra work, but wouldn't
>> look to be otherwise harmful: Considering pages dirty which aren't
>> is never a functional problem, while considering pages clean which
>> aren't is (imo) not a problem for a dying domain.
>
> Can't that lead to failures elsewhere when attempts to fetch those
> pages find they might have been removed from the p2m?
Quite possible, yes.
Jan
Re: [PATCH for-4.17?] x86/paging: return -EINVAL for paging domctls for dying domains
On 09.11.2022 17:11, Andrew Cooper wrote: > On 08/11/2022 11:38, Roger Pau Monne wrote: >> Like on the Arm side, return -EINVAL when attempting to do a p2m >> operation on dying domains. > > Honestly, I'd drop the comment about ARM. "the Arm side" has existed > for of all of a couple of weeks. > > A far better justification is because almost all other DOMCTLs are > rejected with -EINVAL against dying domains. Would you mind supplying data to prove this statement? When looking at just x86'es arch_do_domctl() I can't see this being the case for ioport_permission, getpageframeinfo3, hypercall_init, set_address_size, get_address_size, sendtrigger, bind_pt_irq, unbind_pt_irq, ioport_mapping, set_ext_vcpucontext, and get_ext_vcpucontext. At which point I stopped checking further because in the order they appear in the file these are _all_ except shadow_op and [gs]ethvmcontext*. Am I overlooking hidden checks of ->is_dying? Jan
Re: [linux-5.4 test] 174684: regressions - FAIL
Hi Jan, > On 10 Nov 2022, at 08:18, Jan Beulich wrote: > > On 10.11.2022 03:48, osstest service owner wrote: >> flight 174684 linux-5.4 real [real] >> http://logs.test-lab.xenproject.org/osstest/logs/174684/ >> >> Regressions :-( >> >> Tests which did not succeed and are blocking, >> including tests which could not be run: >> test-armhf-armhf-xl-credit2 18 guest-start/debian.repeat fail REGR. vs. >> 174540 > > This now looks to be failing relatively frequently (about every other flight), > and I'd rather suspect it to point at a hypervisor issue than a Linux one. > Looking at the log I found > > (XEN) d5v0: vGICD: unhandled word write 0x to ICACTIVER0 > (XEN) arch/arm/traps.c:1985:d5v1 HSR=0x8006 pc= gva=0 > gpa= > (XEN) arch/arm/traps.c:1985:d5v1 HSR=0x8006 pc=0x0c gva=0xc > gpa=0x0c > (XEN) arch/arm/traps.c:1985:d5v1 HSR=0x8006 pc=0x0c gva=0xc > gpa=0x0c > > with the last two messages then repeated over and over, many dozen times a > second. Which makes me wonder whether that verbosity alone is causing a > problem. The 2 messages are not necessarily related. The first one is a warning and the write is ignored, we see that all the time with Xen in Debug mode during linux boot. The other traps, with PC at 0xc look like there was an exception in Linux but the register pointing to the exception table is at 0 which means the guest did an exception before it initialised the exception table pointer. When a guest is going to this kind of error it will loop on this: - exception - jump to exception table at 0 + offset depending on exception - trap in xen because 0 is not mapped and push back the exception to guest - go back to step 1 Now it could be that the first exception we reported back to guest should not have been reported and the guest falls into this loop after. Our CI was a bit late compared to staging status until today (due to some servers being down). I will check what is happening on our side with the last status today on arm32. Cheers Bertrand > > Jan
Re: [PATCH for 4.17] arm: fix Kconfig symbol dependency on arm features
Hi Luca,
> On 9 Nov 2022, at 14:04, Luca Fancellu wrote:
>
> The commit 3c2a14ea81c7 is introducing some unsupported arm features
> that by default are disabled and are used for the cpufeature.c code.
>
> As they are disabled by default, a typo in the Kconfig symbol they
> depend on has landed in the codebase unnoticed, instead of depending
> on ARM64 which does not exist, fix the code to depend on ARM_64 that
> is the intended symbol.
>
> Fixes: 3c2a14ea81c7 ("arm: Define kconfig symbols used by arm64 cpufeatures")
> Signed-off-by: Luca Fancellu
Reviewed-by: Bertrand Marquis
I think this should go in 4.17 as it is fixing an invalid depends in Kconfig.
The change cannot create any issue as those config options are hidden and
default to n at the moment.
Cheers
Bertrand
> ---
> xen/arch/arm/Kconfig | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 1fe5faf847b8..52a05f704da5 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -105,28 +105,28 @@ config HARDEN_BRANCH_PREDICTOR
>
> config ARM64_PTR_AUTH
> def_bool n
> - depends on ARM64
> + depends on ARM_64
> help
> Pointer authentication support.
> This feature is not supported in Xen.
>
> config ARM64_SVE
> def_bool n
> - depends on ARM64
> + depends on ARM_64
> help
> Scalar Vector Extension support.
> This feature is not supported in Xen.
>
> config ARM64_MTE
> def_bool n
> - depends on ARM64
> + depends on ARM_64
> help
> Memory Tagging Extension support.
> This feature is not supported in Xen.
>
> config ARM64_BTI
> def_bool n
> - depends on ARM64
> + depends on ARM_64
> help
> Branch Target Identification support.
> This feature is not supported in Xen.
> --
> 2.17.1
>
Re: [PATCH for-4.17 v3 07/15] CODING_STYLE(tools/ocaml): add 'make format' and remove tabs
> On 9 Nov 2022, at 02:40, Henry Wang wrote: > >> >> -Original Message- >> From: Julien Grall >> Subject: Re: [PATCH for-4.17 v3 07/15] CODING_STYLE(tools/ocaml): add >> 'make format' and remove tabs >> While I understand the goal and support, this seems to be a bit too late >> to do it in Xen 4.17 (we are only a couple of weeks away). At this stage >> of the release we should only do bug fix. >> >> This is clearly only a comesmetic change and there I would argue this >> should be deferred to 4.18. That said the last call is from the RM. > > I agree with your point. I think maybe defer the patch to 4.18 is better, > given the deep freeze state we are currently in. I disagree. This is an automated change that can be verified to not add functional changes. Edvin has demonstrated that wrong indentation has mislead reviewers in the past and caused bugs. Nobody except Edvin has contributed to the affected code in years and thus it is not a burden on the project outside the OCaml part. I suggest to accept this. — C
Re: [linux-5.4 test] 174684: regressions - FAIL
On 10.11.2022 03:48, osstest service owner wrote: > flight 174684 linux-5.4 real [real] > http://logs.test-lab.xenproject.org/osstest/logs/174684/ > > Regressions :-( > > Tests which did not succeed and are blocking, > including tests which could not be run: > test-armhf-armhf-xl-credit2 18 guest-start/debian.repeat fail REGR. vs. > 174540 This now looks to be failing relatively frequently (about every other flight), and I'd rather suspect it to point at a hypervisor issue than a Linux one. Looking at the log I found (XEN) d5v0: vGICD: unhandled word write 0x to ICACTIVER0 (XEN) arch/arm/traps.c:1985:d5v1 HSR=0x8006 pc= gva=0 gpa= (XEN) arch/arm/traps.c:1985:d5v1 HSR=0x8006 pc=0x0c gva=0xc gpa=0x0c (XEN) arch/arm/traps.c:1985:d5v1 HSR=0x8006 pc=0x0c gva=0xc gpa=0x0c with the last two messages then repeated over and over, many dozen times a second. Which makes me wonder whether that verbosity alone is causing a problem. Jan
Re: Revert of the 4.17 hypercall handler changes Re: [PATCH-for-4.17] xen: fix generated code for calling hypercall handlers
On 09.11.2022 21:16, George Dunlap wrote:
>> On 4 Nov 2022, at 05:01, Andrew Cooper wrote:
>> On 03/11/2022 16:36, Juergen Gross wrote:
>>> The code generated for the call_handlers_*() macros needs to avoid
>>> undefined behavior when multiple handlers share the same priority.
>>> The issue is the hypercall number being unverified fed into the macros
>>> and then used to set a mask via "mask = 1ULL << ".
>>>
>>> Avoid a shift amount of more than 63 by setting mask to zero in case
>>> the hypercall number is too large.
>>>
>>> Fixes: eca1f00d0227 ("xen: generate hypercall interface related code")
>>> Signed-off-by: Juergen Gross
>>
>> This is not a suitable fix. There being a security issue is just the
>> tip of the iceberg.
>
> At the x86 Maintainer’s meeting on Monday, we (Andrew, Jan, and I) talked
> about this patch. Here is my summary of the conversation (with the caveat
> that I may get some of the details wrong).
Just a couple of remarks, mainly to extend context:
> The proposed benefits of the series are:
>
> 1. By removing indirect calls, it removes those as a “speculative attack
> surface”.
>
> 2. By removing indirect calls, it provides some performance benefit, since
> indirect calls require an extra memory fetch.
>
> 3. It avoids casting function pointers to function pointers of a different
> type. Our current practice is *technically* UB, and is incompatible with
> some hardware safety mechanisms which we may want to take advantage of at
> some point in the future; the series addresses both.
>
> There were two incidental technical problems pointed out:
>
> 1. A potential shift of more than 64 bytes, which is UB; this has been fixed.
>
> 2. The prototype for the kexec_op call was changed from unsigned long to
> unsigned int; this is an ABI change which will cause differing behavior. Jan
> will be looking at how he can fix this, now that it’s been noted.
Patch was already sent and is now fully acked. Will go in later this morning.
> But the more fundamental costs include:
>
> 1. The code is much more difficult now to reason about
>
> 2. The code is much larger
>
> 3. The long if/else chain could theoretically help hypercalls at the top if
> the chain, but would definitely begin to hurt hypercalls at the bottom of the
> chain; and the more hypercalls we add, the more of a theoretical performance
> penalty this will have
After Andrew's remark on how branch history works I've looked at Intel's
ORM, finding that they actually recommend a hybrid approach: Frequently
used numbers dealt with separately, infrequently used ones dealt with by
a common indirect call.
> 4. By using 64-bit masks, the implementation limits the number of hypercalls
> to 64; a number we are likely to exceed if we implement ABIv2 to be
> compatible with AMD SEV.
This very much depends on how we encode the new hypercall numbers. In my
proposal a single bit is used, and handlers remain the same. Therefore in
that model there wouldn't really be an extension of hypercall numbers to
cover here.
> Additionally, there is a question about whether some of the alleged benefits
> actually help:
>
> 1. On AMD processors, we enable IBRS, which completely removes indirect calls
> as a speculative attack surface already. And on Intel processors, this
> attack surface has already been significantly reduced. So removing indirect
> calls is not as important an issue.
>
> 2. Normal branches are *also* a surface of speculative attacks; so even apart
> from the above, all this series does is change one potential attack surface
> for another one.
>
> 3. When we analyze theoretical performance with deep CPU pipelines and
> speculation in mind, the theoretical disadvantage of indirect branches goes
> away; and depending on the hardware, there is a theoretical performance
> degradation.
I'm inclined to change this to "may go away". As Andrew said on the call, an
important criteria for the performance of indirect calls is how long it takes
to recognize misprediction, and hence how much work needs to be thrown away
and re-done. Which in turn means there's a more significant impact here when
the rate of mis-predictions is higher.
> 4. From a practical perspective, the performance tests are very much
> insufficient to show either that this is an improvement, or that does not
> cause a performance regression. To show that there hasn’t been a performance
> degradation, a battery of tests needs to be done on hardware from a variety
> of different vendors and cpu generations, since each of them will have
> different properties after all speculative mitigations have been applied.
>
> So the argument is as follows:
>
> There is no speculative benefit for the series; there is insufficient
> performance evidence, either to justify a performance benefit or to allay
> doubts about a performance regression; and the benefit that there is
> insufficient to counterbalance the costs, and so the series should
