[Bug 1916112] Re: Illegal instruction crash of QEMU on Jetson Nano
Disassembly: [ OK ] Mounted RPC Pipe File System. [ 75.916706] systemd[1]: Started Create list of required static device nodes for the current kernel. [ OK ] Started Create list of req… nodes for the current kernel. Thread 7 "qemu-system-aar" received signal SIGILL, Illegal instruction. [Switching to Thread 0x7fade0aba0 (LWP )] 0x007f8aca04d0 in code_gen_buffer () (gdb) disas $pc-32,$pc+32 Dump of assembler code from 0x7f8aca04b0 to 0x7f8aca04f0: 0x007f8aca04b0 : cmp x0, x3 0x007f8aca04b4 : b.ne0x7f8aca0908 // b.any 0x007f8aca04b8 : ldr x23, [x1, x23] 0x007f8aca04bc : str x23, [x19, #3688] 0x007f8aca04c0 : add w22, w22, w21 0x007f8aca04c4 : str w22, [x19, #16] 0x007f8aca04c8 : ldr d0, [x19, #3944] 0x007f8aca04cc : ldr d1, [x19, #4192] => 0x007f8aca04d0 : .inst 0x2ee0b822 ; undefined 0x007f8aca04d4 : movid3, #0xff 0x007f8aca04d8 : and v1.8b, v1.8b, v3.8b 0x007f8aca04dc : and v2.8b, v2.8b, v3.8b 0x007f8aca04e0 : .inst 0x2ee14404 ; undefined 0x007f8aca04e4 : .inst 0x2ee0b845 ; und--Typ--Ty--Ty-Ty--T--Type--Type-Ty--T--Type for more, q to quit, c to continue without paging-- efined 0x007f8aca04e8 : .inst 0x2ee54400 ; undefined 0x007f8aca04ec : ldr d5, 0x7f8aca09f0 End of assembler dump. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1916112 Title: Illegal instruction crash of QEMU on Jetson Nano Status in QEMU: New Bug description: I have a jetson nano (arm64 SBC) and I want to check the native emulation performance of Raspbian Buster. I used the info available here: https://github.com/dhruvvyas90/qemu-rpi-kernel/tree/master/native- emuation I have Xubuntut 20.04 with KVM enabled kernel running on the Jetson Nano However QEMU crashes with "Illegal Instruction" during kernel boot. I have a built latest QEMU from sources with following configuration ./configure --prefix=/usr/local --target-list=aarch64-softmmu,arm- softmmu --enable-guest-agent --enable-vnc --enable-vnc-jpeg --enable-vnc-png --enable-kvm --enable-spice --enable-sdl --enable-gtk --enable-virglrenderer --enable-opengl qemu-system-aarch64 --version QEMU emulator version 5.2.50 (v5.2.0-1731-g5b19cb63d9) When I run as follows: ../build/qemu-system-aarch64 -M raspi3 -append "rw earlyprintk loglevel=8 console=ttyAMA0,115200 dwc_otg.lpm_enable=0 root=/dev/mmcblk0p2 rootdelay=1" -dtb ./bcm2710-rpi-3-b-plus.dtb -sd /media/96747D21747D0571/JetsonNano/2020-08-20-raspios-buster-armhf-full.qcow2 -kernel ./kernel8.img -m 1G -smp 4 -serial stdio -usb -device usb-mouse -device usb-kbd I get : [ 74.994834] systemd[1]: Condition check resulted in FUSE Control File System being skipped. [ 76.281274] systemd[1]: Starting Apply Kernel Variables... Starting Apply Kernel Variables... Illegal instruction (core dumped) When I use GDB I see this: Thread 8 "qemu-system-aar" received signal SIGILL, Illegal instruction. [Switching to Thread 0x7fad7f9ba0 (LWP 28037)] 0x007f888ac690 in code_gen_buffer () (gdb) bt #0 0x007f888ac690 in code_gen_buffer () #1 0x00d7c038 in cpu_tb_exec (tb_exit=, itb=, cpu=0x7fb4502c40) at ../accel/tcg/cpu-exec.c:191 #2 cpu_loop_exec_tb (tb_exit=, last_tb=, tb=, cpu=0x7fb4502c40) at ../accel/tcg/cpu-exec.c:708 #3 cpu_exec (cpu=cpu@entry=0x7fb4502c40) at ../accel/tcg/cpu-exec.c:819 .. I have just two questions: Is this a problem with QEMU or is there anything specific build or options I need to use. Any specific version of QEMU should be used ? Why is TCG used as the accelerator when KVM is present. Is it possible and how to use KVM ? If I enabled the KVM then I get this error: ../build/qemu-system-aarch64 -M raspi3 -enable-kvm -append "rw earlyprintk loglevel=8 console=ttyAMA0,115200 dwc_otg.lpm_enable=0 root=/dev/mmcblk0p2 rootdelay=1" -dtb ./bcm2710-rpi-3-b-plus.dtb -sd /media/96747D21747D0571/JetsonNano/2020-08-20-raspios-buster-armhf-full.qcow2 -kernel ./kernel8.img -m 1G -smp 4 -serial stdio -usb -device usb-mouse -device usb-kbd WARNING: Image format was not specified for '/media/96747D21747D0571/JetsonNano/2020-08-20-raspios-buster-armhf-full.img' and probing guessed raw. Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted. Specify the 'raw' format explicitly to remove the restrictions. qemu-system-aarch64: ../softmmu/physmem.c:750: cpu_address_space_init: Assertion `asidx == 0 || !kvm_enabled()' failed. Thanks a lot. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1916112/+subscriptions
Re: [PATCH v2 5/6] hw/sd: sdhci: Limit block size only when SDHC_BLKSIZE register is writable
Hi Philippe, On Fri, Feb 19, 2021 at 2:03 AM Philippe Mathieu-Daudé wrote: > > On 2/18/21 6:09 PM, Philippe Mathieu-Daudé wrote: > > On 2/16/21 4:46 AM, Bin Meng wrote: > >> The codes to limit the maximum block size is only necessary when > >> SDHC_BLKSIZE register is writable. > > Per "SD Command Generation": > > The Host Driver should not read the SDMA System Address, Block Size > and Block Count registers during a data transaction unless the > transfer is stopped because the value is changing and not stable. > To prevent destruction of registers using data transfer when issuing > command, the 32-bit Block Count, Block Size, 16-bit Block Count and > Transfer Mode registers shall be write protected by the Host > Controller while Command Inhibit (DAT) is set to 1 in the Present > State register. > > Shouldn't we check for !(s->prnsts & SDHC_DATA_INHIBIT) instead? Yes, for accurate emulation I think we should. Current implementation uses !(s->prnsts & (SDHC_DOING_READ | SDHC_DOING_WRITE)) which eventually is correct, because: SDHC_DATA_INHIBIT bit is set if either SDHC_DAT_LINE_ACTIVE or SDHC_DOING_READ is set (SD Host Controller Spec v7.00 chapter 2.2.9 Present State Register) SDHC_DAT_LINE_ACTIVE bit is set after the end bit of read or write command, and after end bit of read or write command will generate SDHC_DOING_READ or SDHC_DOING_WRITE (SD Host Controller Spec v7.00 chapter 2.2.9 Present State Register) Regards, Bin
Re: [PATCH v4] net/macos: implement vmnet-based netdev
On Thu, Feb 18, 2021 at 2:49 PM wrote: > > From: Phillip Tennen > > This patch implements a new netdev device, reachable via -netdev > vmnet-macos, that’s backed by macOS’s vmnet framework. > > The vmnet framework provides native bridging support, and its usage in > this patch is intended as a replacement for attempts to use a tap device > via the tuntaposx kernel extension. Notably, the tap/tuntaposx approach > never would have worked in the first place, as QEMU interacts with the > tap device via poll(), and macOS does not support polling device files. > > vmnet requires either a special entitlement, granted via a provisioning > profile, or root access. Otherwise attempts to create the virtual > interface will fail with a “generic error” status code. QEMU may not > currently be signed with an entitlement granted in a provisioning > profile, as this would necessitate pre-signed binary build distribution, > rather than source-code distribution. As such, using this netdev > currently requires that qemu be run with root access. I’ve opened a > feedback report with Apple to allow the use of the relevant entitlement > with this use case: > https://openradar.appspot.com/radar?id=5007417364447232 > > vmnet offers three operating modes, all of which are supported by this > patch via the “mode=host|shared|bridge” option: > > * "Host" mode: Allows the vmnet interface to communicate with other > * vmnet > interfaces that are in host mode and also with the native host. > * "Shared" mode: Allows traffic originating from the vmnet interface to > reach the Internet through a NAT. The vmnet interface can also > communicate with the native host. > * "Bridged" mode: Bridges the vmnet interface with a physical network > interface. > > Each of these modes also provide some extra configuration that’s > supported by this patch: > > * "Bridged" mode: The user may specify the physical interface to bridge > with. Defaults to en0. > * "Host" mode / "Shared" mode: The user may specify the DHCP range and > subnet. Allocated by vmnet if not provided. > > vmnet also offers some extra configuration options that are not > supported by this patch: > > * Enable isolation from other VMs using vmnet > * Port forwarding rules > * Enabling TCP segmentation offload > * Only applicable in "shared" mode: specifying the NAT IPv6 prefix > * Only available in "host" mode: specifying the IP address for the VM > within an isolated network > > Note that this patch requires macOS 10.15 as a minimum, as this is when > bridging support was implemented in vmnet.framework. > > Signed-off-by: Phillip Tennen Hi Phillip, Thanks for the updated patch. I have a small problem applying it with either git am or patch. I have to manually fix configure. This has been the case from v1 up to now: hsp@hsps-Catalina-VB qemu-master % patch -p1 < ../patches/qemu/v4-net-macos-implement-vmnet-based-netdev.patch patching file configure Hunk #1 FAILED at 778. 1 out of 1 hunk FAILED -- saving rejects to file configure.rej patching file net/clients.h patching file net/meson.build patching file net/net.c patching file net/vmnet-macos.c patching file qapi/net.json patching file qemu-options.hx Hunk #1 succeeded at 2507 (offset 24 lines). Best, Howard
Re: [PATCH v2 05/11] hw/mips: Restrict KVM to the malta & virt machines
Reviewed-by: Huacai Chen On Sat, Feb 20, 2021 at 12:56 PM Jiaxun Yang wrote: > > 在 2021/2/20 上午1:38, Philippe Mathieu-Daudé 写道: > > Restrit KVM to the following MIPS machines: > > - malta > > - loongson3-virt > > > > Signed-off-by: Philippe Mathieu-Daudé > > Reviewed-by: Jiaxun Yang > > > --- > > hw/mips/loongson3_virt.c | 5 + > > hw/mips/malta.c | 5 + > > 2 files changed, 10 insertions(+) > > > > diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c > > index d4a82fa5367..c3679dff043 100644 > > --- a/hw/mips/loongson3_virt.c > > +++ b/hw/mips/loongson3_virt.c > > @@ -612,6 +612,10 @@ static void mips_loongson3_virt_init(MachineState > > *machine) > > loongson3_virt_devices_init(machine, liointc); > > } > > > > +static const char *const valid_accels[] = { > > +"tcg", "kvm", NULL > > +}; > > + > > static void loongson3v_machine_class_init(ObjectClass *oc, void *data) > > { > > MachineClass *mc = MACHINE_CLASS(oc); > > @@ -622,6 +626,7 @@ static void loongson3v_machine_class_init(ObjectClass > > *oc, void *data) > > mc->max_cpus = LOONGSON_MAX_VCPUS; > > mc->default_ram_id = "loongson3.highram"; > > mc->default_ram_size = 1600 * MiB; > > +mc->valid_accelerators = valid_accels; > > mc->kvm_type = mips_kvm_type; > > mc->minimum_page_bits = 14; > > } > > diff --git a/hw/mips/malta.c b/hw/mips/malta.c > > index 9afc0b427bf..0212048dc63 100644 > > --- a/hw/mips/malta.c > > +++ b/hw/mips/malta.c > > @@ -1443,6 +1443,10 @@ static const TypeInfo mips_malta_device = { > > .instance_init = mips_malta_instance_init, > > }; > > > > +static const char *const valid_accels[] = { > > +"tcg", "kvm", NULL > > +}; > > + > > static void mips_malta_machine_init(MachineClass *mc) > > { > > mc->desc = "MIPS Malta Core LV"; > > @@ -1456,6 +1460,7 @@ static void mips_malta_machine_init(MachineClass *mc) > > mc->default_cpu_type = MIPS_CPU_TYPE_NAME("24Kf"); > > #endif > > mc->default_ram_id = "mips_malta.ram"; > > +mc->valid_accelerators = valid_accels; > > } > > > > DEFINE_MACHINE("malta", mips_malta_machine_init) >
Re: [PATCH] opengl: Do not convert format with glTexImage2D on OpenGL ES
2021年2月19日(金) 23:14 Gerd Hoffmann : > > On Fri, Feb 19, 2021 at 06:48:03PM +0900, Akihiko Odaki wrote: > > OpenGL ES does not support conversion from the given data format > > to the internal format with glTexImage2D. > > > > Use the given data format as the internal format, and ignore > > the given alpha channels with GL_TEXTURE_SWIZZLE_A in case the > > format contains alpha channels. > > Hmm. Do you know what effect this has performance-wise? > Is it maybe useful to not convert for desktop gl too? I have no idea about performance, but I am concerned about compatibility. OpenGL 4.6 core profile does not support GL_BGRA, which is aliased as GL_BGRA_EXT by epoxy, as internalformat. I also tested with Intel HD Graphics 3000/Mesa 20.3.4 but it didn't work. > > take care, > Gerd >
Re: [PATCH] target/riscv: fix TB_FLAGS bits overlapping bug for rvv/rvh
On Sat, Feb 20, 2021 at 12:12 AM Richard Henderson < richard.hender...@linaro.org> wrote: > On 2/19/21 1:59 AM, frank.ch...@sifive.com wrote: > > +/* Skip mem_idx bits */ > > +FIELD(TB_FLAGS, VL_EQ_VLMAX, 3, 1) > > Why not just add the mem_idx field to the list? > > The separation between the FIELDs and TB_FLAG_*_MASK is unfortunate, and > will > be a continuing source of errors. > > Sure, I will edit it and send out the next version patch. Thanks, Frank Chang > > r~ >
Re: [PATCH v2 05/11] hw/mips: Restrict KVM to the malta & virt machines
在 2021/2/20 上午1:38, Philippe Mathieu-Daudé 写道: Restrit KVM to the following MIPS machines: - malta - loongson3-virt Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Jiaxun Yang --- hw/mips/loongson3_virt.c | 5 + hw/mips/malta.c | 5 + 2 files changed, 10 insertions(+) diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c index d4a82fa5367..c3679dff043 100644 --- a/hw/mips/loongson3_virt.c +++ b/hw/mips/loongson3_virt.c @@ -612,6 +612,10 @@ static void mips_loongson3_virt_init(MachineState *machine) loongson3_virt_devices_init(machine, liointc); } +static const char *const valid_accels[] = { +"tcg", "kvm", NULL +}; + static void loongson3v_machine_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); @@ -622,6 +626,7 @@ static void loongson3v_machine_class_init(ObjectClass *oc, void *data) mc->max_cpus = LOONGSON_MAX_VCPUS; mc->default_ram_id = "loongson3.highram"; mc->default_ram_size = 1600 * MiB; +mc->valid_accelerators = valid_accels; mc->kvm_type = mips_kvm_type; mc->minimum_page_bits = 14; } diff --git a/hw/mips/malta.c b/hw/mips/malta.c index 9afc0b427bf..0212048dc63 100644 --- a/hw/mips/malta.c +++ b/hw/mips/malta.c @@ -1443,6 +1443,10 @@ static const TypeInfo mips_malta_device = { .instance_init = mips_malta_instance_init, }; +static const char *const valid_accels[] = { +"tcg", "kvm", NULL +}; + static void mips_malta_machine_init(MachineClass *mc) { mc->desc = "MIPS Malta Core LV"; @@ -1456,6 +1460,7 @@ static void mips_malta_machine_init(MachineClass *mc) mc->default_cpu_type = MIPS_CPU_TYPE_NAME("24Kf"); #endif mc->default_ram_id = "mips_malta.ram"; +mc->valid_accelerators = valid_accels; } DEFINE_MACHINE("malta", mips_malta_machine_init)
[PATCH v3] ui/cocoa: Use kCGColorSpaceSRGB
kCGColorSpaceGenericRGB | Apple Developer Documentation https://developer.apple.com/documentation/coregraphics/kcgcolorspacegenericrgb > Deprecated > Use kCGColorSpaceSRGB instead. This change also removes the legacy color space specification for PowerPC. Signed-off-by: Akihiko Odaki --- ui/cocoa.m | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index 13fba8103e1..7710835c4c1 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -436,13 +436,8 @@ - (void) drawRect:(NSRect) rect screen.bitsPerComponent, //bitsPerComponent screen.bitsPerPixel, //bitsPerPixel (screen.width * (screen.bitsPerComponent/2)), //bytesPerRow -#ifdef __LITTLE_ENDIAN__ -CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), //colorspace for OS X >= 10.4 -kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst, -#else -CGColorSpaceCreateDeviceRGB(), //colorspace for OS X < 10.4 (actually ppc) -kCGImageAlphaNoneSkipFirst, //bitmapInfo -#endif +CGColorSpaceCreateWithName(kCGColorSpaceSRGB), //colorspace +kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst, //bitmapInfo dataProviderRef, //provider NULL, //decode 0, //interpolate -- 2.24.3 (Apple Git-128)
[Bug 1906180] Re: Keyboard keys get stuck
[Expired for QEMU because there has been no activity for 60 days.] ** Changed in: qemu Status: Incomplete => Expired -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1906180 Title: Keyboard keys get stuck Status in QEMU: Expired Bug description: Keyboard keys get "stuck" quite often, on certain Linux guests at least, and start repeating themselves until another key is pressed. This is especially noticeable with key combinations like Ctrl+V for pasting. When it happens, you get the pasted text and v... This bug has been present for quite some time but I don't remember any specific version that had it first. QEMU version: 5.1.0 Guest: Debian stable 64-bit (live), with Gnome desktop (may occur with other Linux guests too) Host: Arch Linux with KDE desktop (X11, wayland not tested); both default and hardened kernel tested QEMU start command: qemu-system-x86_64 -enable-kvm -m 6G -cpu host -smp 3 -cdrom debian.iso -boot d -vga std To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1906180/+subscriptions
Re: [PATCH v2 6/6] hw/sd: sdhci: Reset the data pointer of s->fifo_buffer[] when a different block size is programmed
Hi Philippe, On Fri, Feb 19, 2021 at 2:06 AM Philippe Mathieu-Daudé wrote: > > Hi Bin, > > On 2/16/21 4:46 AM, Bin Meng wrote: > > If the block size is programmed to a different value from the > > previous one, reset the data pointer of s->fifo_buffer[] so that > > s->fifo_buffer[] can be filled in using the new block size in > > the next transfer. > > > > With this fix, the following reproducer: > > > > outl 0xcf8 0x80001010 > > outl 0xcfc 0xe000 > > outl 0xcf8 0x80001001 > > outl 0xcfc 0x0600 > > write 0xe02c 0x1 0x05 > > write 0xe005 0x1 0x02 > > write 0xe007 0x1 0x01 > > write 0xe028 0x1 0x10 > > write 0x0 0x1 0x23 > > write 0x2 0x1 0x08 > > write 0xe00c 0x1 0x01 > > write 0xe00e 0x1 0x20 > > write 0xe00f 0x1 0x00 > > write 0xe00c 0x1 0x32 > > write 0xe004 0x2 0x0200 > > write 0xe028 0x1 0x00 > > write 0xe003 0x1 0x40 > > > > cannot be reproduced with the following QEMU command line: > > > > $ qemu-system-x86_64 -nographic -machine accel=qtest -m 512M \ > > -nodefaults -device sdhci-pci,sd-spec-version=3 \ > > -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive \ > > -device sd-card,drive=mydrive -qtest stdio > > > > Cc: qemu-sta...@nongnu.org > > Fixes: CVE-2020-17380 > > Fixes: CVE-2020-25085 > > Fixes: CVE-2021-3409 > > Fixes: d7dfca0807a0 ("hw/sdhci: introduce standard SD host controller") > > Reported-by: Alexander Bulekov > > Reported-by: Cornelius Aschermann (Ruhr-University Bochum) > > Reported-by: Muhammad Ramdhan > > Reported-by: Sergej Schumilo (Ruhr-University Bochum) > > Reported-by: Simon Wrner (Ruhr-University Bochum) > > Buglink: https://bugs.launchpad.net/qemu/+bug/1892960 > > Buglink: https://bugs.launchpad.net/qemu/+bug/1909418 > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1928146 > > Signed-off-by: Bin Meng > > > > --- > > > > Changes in v2: > > - new patch: sdhci: Reset the data pointer of s->fifo_buffer[] when a > > different block size is programmed > > > > hw/sd/sdhci.c | 12 > > 1 file changed, 12 insertions(+) > > > > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c > > index d0c8e29..5b86781 100644 > > --- a/hw/sd/sdhci.c > > +++ b/hw/sd/sdhci.c > > @@ -1140,6 +1140,8 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t > > val, unsigned size) > > break; > > case SDHC_BLKSIZE: > > if (!TRANSFERRING_DATA(s->prnsts)) { > > +uint16_t blksize = s->blksize; > > + > > MASKED_WRITE(s->blksize, mask, extract32(value, 0, 12)); > > MASKED_WRITE(s->blkcnt, mask >> 16, value >> 16); > > > > @@ -1151,6 +1153,16 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t > > val, unsigned size) > > > > s->blksize = deposit32(s->blksize, 0, 12, s->buf_maxsz); > > } > > + > > +/* > > + * If the block size is programmed to a different value from > > + * the previous one, reset the data pointer of s->fifo_buffer[] > > + * so that s->fifo_buffer[] can be filled in using the new > > block > > + * size in the next transfer. > > + */ > > +if (blksize != s->blksize) { > > +s->data_count = 0; > > I doubt the hardware works that way. Me too, because s->data_count is not exposed by the hardware as a register or descriptor, so it's purely our internal implementation. A hardware might implement like that, but we really don't know unless some hardware guys who designed a SDHC could jump out and comment :) > Shouldn't we reset the FIFO each time BLKSIZE is accessed, regardless of its > previous value? If we do that, we will end up rewriting the logic of the data transfer functions. I looked at the current implementation and I think there are some spec violations about handling page boundaries, and that part is related to sd->data-count. But like I said in the cover letter these should be addressed in future patches. > > > +} > > } > > > > break; > > Regards, Bin
Re: [PATCH] net: eepro100: validate various address values
On 210219 1243, Li Qiang wrote: > Alexander Bulekov 于2021年2月19日周五 上午10:15写道: > > > > On 210219 1006, Li Qiang wrote: > > > Alexander Bulekov 于2021年2月19日周五 上午9:56写道: > > > > > > > > On 210218 1441, Peter Maydell wrote: > > > > > On Thu, 18 Feb 2021 at 14:13, P J P wrote: > > > > > > > > > > > > From: Prasad J Pandit > > > > > > > > > > > > While processing controller commands, eepro100 emulator gets > > > > > > command unit(CU) base address OR receive unit (RU) base address > > > > > > OR command block (CB) address from guest. If these values are not > > > > > > checked, it may lead to an infinite loop kind of issues. Add checks > > > > > > to avoid it. > > > > > > > > > So could you please provide a backtrack? > > > > > > > I don't know if you are asking me or Prasad, but here is the stacktrace > > > Yes, a typical DMA reentry issue. > Any progress to solve these DMA reentry issues? seems more and more > this kind of issues. Unfortuantely, I don't think there's a solution yet. > Just return from the busy things as a new father and not focus this > quite a time. Congrats! > > Thanks, > Li Qiang > > > for the one I provided: > > ==2715275==ERROR: AddressSanitizer: stack-overflow on address > > 0x7ffc5262ba28 (pc 0x55d83b103ac6 bp 0x7ffc5262c270 sp 0x7ffc5262ba30 > > T0) > > #0 in __asan_memcpy (qemu-system-i386+0x2aa3ac6) > > #1 in flatview_do_translate ../softmmu/physmem.c:518:12 > > #2 in flatview_translate ../softmmu/physmem.c:568:15 > > #3 in flatview_read ../softmmu/physmem.c:2878:10 > > #4 in address_space_read_full ../softmmu/physmem.c:2892:18 > > #5 in dma_memory_rw_relaxed include/sysemu/dma.h:88:12 > > #6 in dma_memory_rw include/sysemu/dma.h:127:12 > > #7 in pci_dma_rw include/hw/pci/pci.h:803:12 > > #8 in pci_dma_read include/hw/pci/pci.h:821:12 > > #9 in read_cb ../hw/net/eepro100.c:726:5 > > #10 in action_command ../hw/net/eepro100.c:847:9 > > #11 in eepro100_cu_command ../hw/net/eepro100.c:969:13 > > #12 in eepro100_write_command ../hw/net/eepro100.c:1063:5 > > #13 in eepro100_write2 ../hw/net/eepro100.c:1510:9 > > #14 in eepro100_write ../hw/net/eepro100.c:1593:9 > > #15 in memory_region_write_accessor ../softmmu/memory.c:491:5 > > #16 in access_with_adjusted_size ../softmmu/memory.c:552:18 > > #17 in memory_region_dispatch_write ../softmmu/memory.c > > #18 in flatview_write_continue ../softmmu/physmem.c:2776:23 > > #19 in flatview_write ../softmmu/physmem.c:2816:14 > > #20 in address_space_write ../softmmu/physmem.c:2908:18 > > #21 in dma_memory_rw_relaxed include/sysemu/dma.h:88:12 > > #22 in dma_memory_rw include/sysemu/dma.h:127:12 > > #23 in dma_memory_write include/sysemu/dma.h:163:12 > > #24 in stw_le_dma include/sysemu/dma.h:259:1 > > #25 in stw_le_pci_dma include/hw/pci/pci.h:855:1 > > #26 in action_command ../hw/net/eepro100.c:913:9 > > #27 in eepro100_cu_command ../hw/net/eepro100.c:969:13 > > #28 in eepro100_write_command ../hw/net/eepro100.c:1063:5 > > #29 in eepro100_write2 ../hw/net/eepro100.c:1510:9 > > #30 in eepro100_write ../hw/net/eepro100.c:1593:9 > > ... till there's no more stack ... > > > > > > > > Thanks, > > > Li Qiang > > > > > > > > > > > > > > > Reported-by: Ruhr-University Bochum > > > > > > Signed-off-by: Prasad J Pandit > > > > > > --- > > > > > > hw/net/eepro100.c | 8 +++- > > > > > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c > > > > > > index 16e95ef9cc..afa1c9b2aa 100644 > > > > > > --- a/hw/net/eepro100.c > > > > > > +++ b/hw/net/eepro100.c > > > > > > @@ -843,7 +843,8 @@ static void action_command(EEPRO100State *s) > > > > > > bool bit_i; > > > > > > bool bit_nc; > > > > > > uint16_t ok_status = STATUS_OK; > > > > > > -s->cb_address = s->cu_base + s->cu_offset; > > > > > > +s->cb_address = s->cu_base + s->cu_offset; /* uint32_t > > > > > > overflow */ > > > > > > +assert (s->cb_address >= s->cu_base); > > > > > > > > > > We get these values from the guest; you can't just assert() on them. > > > > > You need to do something else. > > > > > > > > > > My reading of the 8255x data sheet is that there is nothing > > > > > in the hardware that forbids the guest from programming the > > > > > device such that the cu_base + cu_offset wraps around: > > > > > http://www.intel.com/content/dam/doc/manual/8255x-10-100-mbps-ethernet-controller-software-dev-manual.pdf > > > > > -- page 30 says that this is all doing 32-bit arithmetic > > > > > on addresses and doesn't say that there is any special case > > > > > handling by the device of overflow of that addition. > > > > > > > > > > Your commit message isn't very clear about what the failure > > > > > case is here, but I think the fix has to be something > > > > > different from this. > > > > > > > > Maybe the infinite loop mentioned in the commit message is actually a > > > > DMA recursion issue? I'm providing a reproducer for a DMA re-entracy > > > > issue below.
Re: [RFC PATCH 3/5] tests: add a sdhci reproducer
On 210219 2306, Philippe Mathieu-Daudé wrote: > On 2/18/21 10:12 PM, Alexander Bulekov wrote: > > This patch serves as an example of a file generated with the > > ./scripts/oss-fuzz/output_reproducer.py script: > > The source file in this patch was generated like this: > > > > $ wget https://paste.debian.net/plain/118513 -O /tmp/trace > > $ export QEMU_ARGS="-nographic -machine accel=qtest -m 512M \ > > -nodefaults -device sdhci-pci,sd-spec-version=3 -drive \ > > if=sd,index=0,file=null-co://,format=raw,id=mydrive \ > > -device sd-card,drive=mydrive -qtest stdio" > > $ export QEMU_PATH=./qemu-system-i386 > > $ ./scripts/oss-fuzz/output_reproducer.py \ > > -owner "Alexander Bulekov " /tmp/trace | \ > > clang-format -style="{BasedOnStyle: llvm, IndentWidth: 4, \ > > ColumnLimit: 90, BreakBeforeBraces: Linux}" > ../tests/qtest/fuzz-sdhci.c > > > > Signed-off-by: Alexander Bulekov > > --- > > tests/qtest/fuzz-sdhci.c | 90 > > tests/qtest/meson.build | 2 + > > 2 files changed, 92 insertions(+) > > create mode 100644 tests/qtest/fuzz-sdhci.c > ... > > > diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build > > index c83bc211b6..97caf84443 100644 > > --- a/tests/qtest/meson.build > > +++ b/tests/qtest/meson.build > > @@ -56,6 +56,8 @@ qtests_i386 = \ > > 'rtc-test', > > 'i440fx-test', > > 'fuzz-test', > > + 'fuzz-sdhci', > > + 'sdhci-test', > > This line ^ belongs to the next patch. I think the line doesn't belong at all. The next patch justs adds to fuzz-sdhci.c > > > 'fw_cfg-test', > > 'device-plug-test', > > 'drive_del-test', > >
Re: [PATCH v2 6/8] hw/sd: sd: Actually perform the erase operation
On Sat, Feb 20, 2021 at 6:28 AM Philippe Mathieu-Daudé wrote: > > On 2/16/21 4:02 PM, Bin Meng wrote: > > From: Bin Meng > > > > At present the sd_erase() does not erase the requested range of card > > data to 0xFFs. Let's make the erase operation actually happen. > > > > Signed-off-by: Bin Meng > > > > --- > > > > Changes in v2: > > - honor the write protection bits for SDSC cards > > > > hw/sd/sd.c | 22 ++ > > 1 file changed, 14 insertions(+), 8 deletions(-) > > > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > > index f1f98bdec3..b386f16fcb 100644 > > --- a/hw/sd/sd.c > > +++ b/hw/sd/sd.c > > @@ -766,6 +766,9 @@ static void sd_erase(SDState *sd) > > uint64_t erase_start = sd->erase_start; > > uint64_t erase_end = sd->erase_end; > > bool sdsc = true; > > +uint64_t wpnum; > > +uint64_t erase_addr; > > +int erase_len = 1 << HWBLOCK_SHIFT; > > > > trace_sdcard_erase(sd->erase_start, sd->erase_end); > > if (sd->erase_start == INVALID_ADDRESS > > @@ -794,17 +797,20 @@ static void sd_erase(SDState *sd) > > sd->erase_end = INVALID_ADDRESS; > > sd->csd[14] |= 0x40; > > > > -/* Only SDSC cards support write protect groups */ > > -if (sdsc) { > > -erase_start = sd_addr_to_wpnum(erase_start); > > -erase_end = sd_addr_to_wpnum(erase_end); > > - > > -for (i = erase_start; i <= erase_end; i++) { > > -assert(i < sd->wpgrps_size); > > -if (test_bit(i, sd->wp_groups)) { > > +memset(sd->data, 0xff, erase_len); > > +erase_addr = erase_start; > > +for (i = 0; i <= (erase_end - erase_start) / erase_len; i++) { > > +if (sdsc) { > > +/* Only SDSC cards support write protect groups */ > > +wpnum = sd_addr_to_wpnum(erase_addr); > > +assert(wpnum < sd->wpgrps_size); > > +if (test_bit(wpnum, sd->wp_groups)) { > > sd->card_status |= WP_ERASE_SKIP; > > +continue; > > So if a group is protected, you skip it but don't increase erase_addr. > If G#4 is protected and G#5 isn't, when you check G#5 you end erasing > G#4. > Oops, good catch! I will send v2. > > } > > } > > +BLK_WRITE_BLOCK(erase_addr, erase_len); > > +erase_addr += erase_len; > > } > > } Regards, Bin
Re: [PATCH v2 3/4] hw/riscv: virt: Limit RAM size in a 32-bit system
Hi Alistair, On Fri, Feb 19, 2021 at 11:39 PM Bin Meng wrote: > > From: Bin Meng > > RV32 supports 34-bit physical address hence the maximum RAM size > should be limitted. Limit the RAM size to 10 GiB, which leaves > some room for PCIe high mmio space. > > For 32-bit host, this is not needed as machine->ram_size cannot > represent a RAM size that big. Use a #if size test to only do > the size limitation for the 64-bit host. > > Signed-off-by: Bin Meng > > --- > > Changes in v2: > - Use a #if size test to only do the size limitation for the 64-bit host > > hw/riscv/virt.c | 10 ++ > 1 file changed, 10 insertions(+) > With the v2, all 32-bit host builds in the CI pipelines passed. Regards, Bin
[PATCH v2] ui/cocoa: Remove the uses of full screen APIs
The detections of [NSView -enterFullScreen:] and [NSView -exitFullScreen:] were wrong. A detection is coded as: [NSView respondsToSelector:@selector(exitFullScreenModeWithOptions:)] but it should be: [NSView instancesRespondToSelector:@selector(exitFullScreenModeWithOptions:)] Because of those APIs were not detected, ui/cocoa always falled back to a borderless window whose frame matches the screen to implement fullscreen behavior. The code using [NSView -enterFullScreen:] and [NSView -exitFullScreen:] will be used if you fix the detections, but its behavior is undesirable; the full screen view stretches the video, changing the aspect ratio, even if zooming is disabled. This change removes the code as it does nothing good. Signed-off-by: Akihiko Odaki --- ui/cocoa.m | 41 +++-- 1 file changed, 15 insertions(+), 26 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index 13fba8103e1..36e45cd98b4 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -564,37 +564,26 @@ - (void) toggleFullScreen:(id)sender isFullscreen = FALSE; [self ungrabMouse]; [self setContentDimensions]; -if ([NSView respondsToSelector:@selector(exitFullScreenModeWithOptions:)]) { // test if "exitFullScreenModeWithOptions" is supported on host at runtime -[self exitFullScreenModeWithOptions:nil]; -} else { -[fullScreenWindow close]; -[normalWindow setContentView: self]; -[normalWindow makeKeyAndOrderFront: self]; -[NSMenu setMenuBarVisible:YES]; -} +[fullScreenWindow close]; +[normalWindow setContentView: self]; +[normalWindow makeKeyAndOrderFront: self]; +[NSMenu setMenuBarVisible:YES]; } else { // switch from desktop to fullscreen isFullscreen = TRUE; [normalWindow orderOut: nil]; /* Hide the window */ [self grabMouse]; [self setContentDimensions]; -if ([NSView respondsToSelector:@selector(enterFullScreenMode:withOptions:)]) { // test if "enterFullScreenMode:withOptions" is supported on host at runtime -[self enterFullScreenMode:[NSScreen mainScreen] withOptions:[NSDictionary dictionaryWithObjectsAndKeys: -[NSNumber numberWithBool:NO], NSFullScreenModeAllScreens, -[NSDictionary dictionaryWithObjectsAndKeys:[NSNumber numberWithBool:NO], kCGDisplayModeIsStretched, nil], NSFullScreenModeSetting, - nil]]; -} else { -[NSMenu setMenuBarVisible:NO]; -fullScreenWindow = [[NSWindow alloc] initWithContentRect:[[NSScreen mainScreen] frame] -styleMask:NSWindowStyleMaskBorderless -backing:NSBackingStoreBuffered -defer:NO]; -[fullScreenWindow setAcceptsMouseMovedEvents: YES]; -[fullScreenWindow setHasShadow:NO]; -[fullScreenWindow setBackgroundColor: [NSColor blackColor]]; -[self setFrame:NSMakeRect(cx, cy, cw, ch)]; -[[fullScreenWindow contentView] addSubview: self]; -[fullScreenWindow makeKeyAndOrderFront:self]; -} +[NSMenu setMenuBarVisible:NO]; +fullScreenWindow = [[NSWindow alloc] initWithContentRect:[[NSScreen mainScreen] frame] +styleMask:NSWindowStyleMaskBorderless +backing:NSBackingStoreBuffered +defer:NO]; +[fullScreenWindow setAcceptsMouseMovedEvents: YES]; +[fullScreenWindow setHasShadow:NO]; +[fullScreenWindow setBackgroundColor: [NSColor blackColor]]; +[self setFrame:NSMakeRect(cx, cy, cw, ch)]; +[[fullScreenWindow contentView] addSubview: self]; +[fullScreenWindow makeKeyAndOrderFront:self]; } } -- 2.24.3 (Apple Git-128)
Re: [PATCH v5 0/5] Add support for ipv6 host forwarding
Patchew URL: https://patchew.org/QEMU/20210220001322.1311139-1-...@google.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210220001322.1311139-1-...@google.com Subject: [PATCH v5 0/5] Add support for ipv6 host forwarding === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/20210220001322.1311139-1-...@google.com -> patchew/20210220001322.1311139-1-...@google.com Switched to a new branch 'test' 9d33831 net: Extend host forwarding to support IPv6 2b79933 net/slirp.c: Refactor address parsing 5090008 inet_parse_host_and_addr: Recognize []:port (empty ipv6 address) 5c2dcad util/qemu-sockets.c: Split host:port parsing out of inet_parse 79b77c4 slirp: Advance libslirp submodule to add ipv6 host-forward support === OUTPUT BEGIN === 1/5 Checking commit 79b77c431b30 (slirp: Advance libslirp submodule to add ipv6 host-forward support) ERROR: Author email address is mangled by the mailing list #2: Author: Doug Evans via total: 1 errors, 0 warnings, 2 lines checked Patch 1/5 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 2/5 Checking commit 5c2dcad2990c (util/qemu-sockets.c: Split host:port parsing out of inet_parse) ERROR: Author email address is mangled by the mailing list #2: Author: Doug Evans via total: 1 errors, 0 warnings, 117 lines checked Patch 2/5 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 3/5 Checking commit 509000883fbd (inet_parse_host_and_addr: Recognize []:port (empty ipv6 address)) ERROR: Author email address is mangled by the mailing list #2: Author: Doug Evans via total: 1 errors, 0 warnings, 20 lines checked Patch 3/5 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 4/5 Checking commit 2b7993354518 (net/slirp.c: Refactor address parsing) ERROR: Author email address is mangled by the mailing list #2: Author: Doug Evans via WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #248: new file mode 100644 WARNING: line over 80 characters #334: FILE: tests/acceptance/hostfwd.py:82: + "host address: error parsing port in address ':')\r\n") total: 1 errors, 2 warnings, 315 lines checked Patch 4/5 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 5/5 Checking commit 9d3383170e32 (net: Extend host forwarding to support IPv6) ERROR: Author email address is mangled by the mailing list #2: Author: Doug Evans via WARNING: line over 80 characters #225: FILE: tests/acceptance/hostfwd.py:101: +self.assertEquals(self.hmc('hostfwd_add vnet tcp:[::1]:65022-[fe80::1]:22'), WARNING: line over 80 characters #228: FILE: tests/acceptance/hostfwd.py:104: + 'host forwarding rule for tcp:[::1]:65022 removed\r\n') WARNING: line over 80 characters #236: FILE: tests/acceptance/hostfwd.py:112: + 'host forwarding rule for udp:[::1]:65042 removed\r\n') WARNING: line over 80 characters #254: FILE: tests/acceptance/hostfwd.py:130: + 'host forwarding rule for udp:[::1]:65042 removed\r\n') WARNING: line over 80 characters #256: FILE: tests/acceptance/hostfwd.py:132: + 'host forwarding rule for udp:[::1]:65042 not found\r\n') WARNING: line over 80 characters #266: FILE: tests/acceptance/hostfwd.py:142: + "(For host address: error parsing IPv6 address '[::1')\r\n") WARNING: line over 80 characters #276: FILE: tests/acceptance/hostfwd.py:152: + "(For host address: error parsing IPv6 address '[::1]')\r\n") WARNING: line over 80 characters #279: FILE: tests/acceptance/hostfwd.py:155: + "(For guest address: error parsing IPv6 address '[foo]')\r\n") WARNING: line over 80 characters #285: FILE: tests/acceptance/hostfwd.py:161: + "':[::1]:66-[fe80::1]:-1' (For guest address: Bad port)\r\n") WARNING: line over 80 characters #288: FILE: tests/acceptance/hostfwd.py:164: + "':[::1]:66-[fe80::1]:6' (For guest address: Bad port)\r\n") WARNING: line over 80 characters #291: FILE: tests/acceptance/hostfwd.py:167: + "':[::1]:66-[fe80::1]:0' (For guest address: Bad port)\r\n") total: 1 errors, 11 warnings, 260 lines
[Bug 1910586] Re: SD card size constraint conceptually wrong
** Changed in: qemu Status: New => Confirmed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1910586 Title: SD card size constraint conceptually wrong Status in QEMU: Confirmed Bug description: The patch discussed here: https://www.mail-archive.com/qemu-devel@nongnu.org/msg720833.html introduces an artificial size constraint for SD cards that has no relation to reality. I'm trying to use an _actual_ **physical** SD card, and qemu tells me its size is "invalid". Something here appears to be conceptually wrong. -- # fdisk -l /dev/sdg Disk /dev/sdg: 14.84 GiB, 15931539456 bytes, 31116288 sectors Disk model: USB SD Reader Units: sectors of 1 * 512 = 512 bytes Sector size (logical/physical): 512 bytes / 512 bytes I/O size (minimum/optimal): 512 bytes / 512 bytes Disklabel type: dos Disk identifier: 0x7a0c8bb0 Device Boot Start End Sectors Size Id Type /dev/sdg1 2048 524287 522240 255M c W95 FAT32 (LBA) /dev/sdg2 524288 31116287 30592000 14.6G 83 Linux # qemu-system-aarch64 -M raspi3 -m 1G -kernel vmlinuz-5.4.79-v8 -dtb bcm2837-rpi-3-b-plus.dtb -append console=ttyAMA0\ root=/dev/mmcblk0p2\ rw -nographic -serial mon:stdio -drive file=/dev/sdg,format=raw qemu-system-aarch64: Invalid SD card size: 14.8 GiB SD card size has to be a power of 2, e.g. 16 GiB. You can resize disk images with 'qemu-img resize ' (note that this will lose data if you make the image smaller than it currently is). -- The same invocation with a dump of the actual image resized to match qemu's odd expectations works fine. This is on QEMU 5.2.0, as evidenced by the following: -- # qemu-system-aarch64 -version QEMU emulator version 5.2.0 Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers -- Is there a simple workaround that disables this rather arbitrary constraint? To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1910586/+subscriptions
Re: [PATCH v5 1/5] slirp: Advance libslirp submodule to add ipv6 host-forward support
Hi Doug, On 2/20/21 1:13 AM, Doug Evans via wrote: When updating submodules, the commit description is a good good place to include the output of: $ git shortlog 8f43a99..26ae658 See for example QEMU commit f350d78f102 ("Update SLOF"). Anyhow up to the maintainer merging your patch. > Signed-off-by: Doug Evans > --- > > Changes from v4: > NOTE TO REVIEWERS: I need some hand-holding to know what The Right > way to submit this particular patch is. > > - no change > > Changes from v3: > - pick up latest libslirp patch to reject ipv6 addr-any for guest address > - libslirp currently only provides a stateless DHCPv6 server, which means > it can't know in advance what the guest's IP address is, and thus > cannot do the "addr-any -> guest ip address" translation that is done > for ipv4 > > Changes from v2: > - this patch is new in v3, split out from v2 > > slirp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/slirp b/slirp > index 8f43a99191..26ae658a83 16 > --- a/slirp > +++ b/slirp > @@ -1 +1 @@ > -Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece > +Subproject commit 26ae658a83eeca16780cf5615c8247cbb151c3fa >
[PULL 14/18] hw/sd: sd: Skip write protect groups check in sd_erase() for high capacity cards
From: Bin Meng High capacity cards don't support write protection hence we should not perform the write protect groups check in sd_erase() for them. Signed-off-by: Bin Meng Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210216150225.27996-6-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 4c6e7c2a33e..883c04de028 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -765,6 +765,7 @@ static void sd_erase(SDState *sd) int i; uint64_t erase_start = sd->erase_start; uint64_t erase_end = sd->erase_end; +bool sdsc = true; trace_sdcard_erase(sd->erase_start, sd->erase_end); if (sd->erase_start == INVALID_ADDRESS @@ -779,6 +780,7 @@ static void sd_erase(SDState *sd) /* High capacity memory card: erase units are 512 byte blocks */ erase_start *= 512; erase_end *= 512; +sdsc = false; } if (erase_start > sd->size || erase_end > sd->size) { @@ -788,16 +790,20 @@ static void sd_erase(SDState *sd) return; } -erase_start = sd_addr_to_wpnum(erase_start); -erase_end = sd_addr_to_wpnum(erase_end); sd->erase_start = INVALID_ADDRESS; sd->erase_end = INVALID_ADDRESS; sd->csd[14] |= 0x40; -for (i = erase_start; i <= erase_end; i++) { -assert(i < sd->wpgrps_size); -if (test_bit(i, sd->wp_groups)) { -sd->card_status |= WP_ERASE_SKIP; +/* Only SDSC cards support write protect groups */ +if (sdsc) { +erase_start = sd_addr_to_wpnum(erase_start); +erase_end = sd_addr_to_wpnum(erase_end); + +for (i = erase_start; i <= erase_end; i++) { +assert(i < sd->wpgrps_size); +if (test_bit(i, sd->wp_groups)) { +sd->card_status |= WP_ERASE_SKIP; +} } } } -- 2.26.2
[PULL 18/18] MAINTAINERS: Add Bin Meng as co-maintainer for SD/MMC cards
There is new interest in the SD/MMC device emulation, so it would be good to have more than only one maintainer / reviewer for it. Bin Meng proved by his contributions a deep understanding of the SD cards internals, so let's add him to the corresponding section in the MAINTAINERS file. Signed-off-by: Philippe Mathieu-Daudé Acked-by: Bin Meng Message-Id: <20210216132841.1121653-1-f4...@amsat.org> --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 66354e6e495..5eeba79c5a3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1754,6 +1754,7 @@ F: hw/ssi/xilinx_* SD (Secure Card) M: Philippe Mathieu-Daudé +M: Bin Meng L: qemu-bl...@nongnu.org S: Odd Fixes F: include/hw/sd/sd* -- 2.26.2
[PULL 12/18] hw/sd: sd: Fix CMD30 response type
From: Bin Meng Per the "Physical Layer Specification Version 8.00", table 4-26 (SD mode) and table 7-3 (SPI mode) command descriptions, CMD30 response type is R1, not R1b. Fixes: a1bb27b1e98a ("SD card emulation initial implementation") Signed-off-by: Bin Meng Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210216150225.27996-4-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index dd1ce0bdae4..47ac0c51a8e 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -1340,7 +1340,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) *(uint32_t *) sd->data = sd_wpbits(sd, req.arg); sd->data_start = addr; sd->data_offset = 0; -return sd_r1b; +return sd_r1; default: break; -- 2.26.2
[PULL 17/18] hw/sd: sdhci: Simplify updating s->prnsts in sdhci_sdma_transfer_multi_blocks()
From: Bin Meng s->prnsts is updated in both branches of the if () else () statement. Move the common bits outside so that it is cleaner. Signed-off-by: Bin Meng Tested-by: Alexander Bulekov Reviewed-by: Alexander Bulekov Reviewed-by: Philippe Mathieu-Daudé Message-Id: <1613447214-81951-5-git-send-email-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sdhci.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index 8ffa53999d8..9acf4467a32 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -596,9 +596,9 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s) page_aligned = true; } +s->prnsts |= SDHC_DATA_INHIBIT | SDHC_DAT_LINE_ACTIVE; if (s->trnmod & SDHC_TRNS_READ) { -s->prnsts |= SDHC_DOING_READ | SDHC_DATA_INHIBIT | -SDHC_DAT_LINE_ACTIVE; +s->prnsts |= SDHC_DOING_READ; while (s->blkcnt) { if (s->data_count == 0) { sdbus_read_data(>sdbus, s->fifo_buffer, block_size); @@ -625,8 +625,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s) } } } else { -s->prnsts |= SDHC_DOING_WRITE | SDHC_DATA_INHIBIT | -SDHC_DAT_LINE_ACTIVE; +s->prnsts |= SDHC_DOING_WRITE; while (s->blkcnt) { begin = s->data_count; if (((boundary_count + begin) < block_size) && page_aligned) { -- 2.26.2
[PULL 11/18] hw/sd: sd: Only SDSC cards support CMD28/29/30
From: Bin Meng Per the "Physical Layer Specification Version 8.00", table 4-26 (SD mode) and table 7-3 (SPI mode) command descriptions, the following commands: - CMD28 (SET_WRITE_PROT) - CMD29 (CLR_WRITE_PROT) - CMD30 (SEND_WRITE_PROT) are only supported by SDSC cards. Signed-off-by: Bin Meng Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210216150225.27996-3-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 12 1 file changed, 12 insertions(+) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 7adcb4edfaa..dd1ce0bdae4 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -1284,6 +1284,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) /* Write protection (Class 6) */ case 28: /* CMD28: SET_WRITE_PROT */ +if (sd->size > SDSC_MAX_CAPACITY) { +return sd_illegal; +} + switch (sd->state) { case sd_transfer_state: if (addr >= sd->size) { @@ -1303,6 +1307,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) break; case 29: /* CMD29: CLR_WRITE_PROT */ +if (sd->size > SDSC_MAX_CAPACITY) { +return sd_illegal; +} + switch (sd->state) { case sd_transfer_state: if (addr >= sd->size) { @@ -1322,6 +1330,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) break; case 30: /* CMD30: SEND_WRITE_PROT */ +if (sd->size > SDSC_MAX_CAPACITY) { +return sd_illegal; +} + switch (sd->state) { case sd_transfer_state: sd->state = sd_sendingdata_state; -- 2.26.2
[PULL 13/18] hw/sd: sd: Move the sd_block_{read, write} and macros ahead
From: Bin Meng These APIs and macros may be referenced by functions that are currently before them. Move them ahead a little bit. Signed-off-by: Bin Meng Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210216150225.27996-5-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 42 +- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 47ac0c51a8e..4c6e7c2a33e 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -739,6 +739,27 @@ void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert) qemu_set_irq(insert, sd->blk ? blk_is_inserted(sd->blk) : 0); } +static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len) +{ +trace_sdcard_read_block(addr, len); +if (!sd->blk || blk_pread(sd->blk, addr, sd->data, len) < 0) { +fprintf(stderr, "sd_blk_read: read error on host side\n"); +} +} + +static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len) +{ +trace_sdcard_write_block(addr, len); +if (!sd->blk || blk_pwrite(sd->blk, addr, sd->data, len, 0) < 0) { +fprintf(stderr, "sd_blk_write: write error on host side\n"); +} +} + +#define BLK_READ_BLOCK(a, len) sd_blk_read(sd, a, len) +#define BLK_WRITE_BLOCK(a, len) sd_blk_write(sd, a, len) +#define APP_READ_BLOCK(a, len) memset(sd->data, 0xec, len) +#define APP_WRITE_BLOCK(a, len) + static void sd_erase(SDState *sd) { int i; @@ -1754,27 +1775,6 @@ send_response: return rsplen; } -static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len) -{ -trace_sdcard_read_block(addr, len); -if (!sd->blk || blk_pread(sd->blk, addr, sd->data, len) < 0) { -fprintf(stderr, "sd_blk_read: read error on host side\n"); -} -} - -static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len) -{ -trace_sdcard_write_block(addr, len); -if (!sd->blk || blk_pwrite(sd->blk, addr, sd->data, len, 0) < 0) { -fprintf(stderr, "sd_blk_write: write error on host side\n"); -} -} - -#define BLK_READ_BLOCK(a, len) sd_blk_read(sd, a, len) -#define BLK_WRITE_BLOCK(a, len)sd_blk_write(sd, a, len) -#define APP_READ_BLOCK(a, len) memset(sd->data, 0xec, len) -#define APP_WRITE_BLOCK(a, len) - void sd_write_byte(SDState *sd, uint8_t value) { int i; -- 2.26.2
[PULL 10/18] hw/sd: sd: Fix address check in sd_erase()
From: Bin Meng For high capacity memory cards, the erase start address and end address are multiplied by 512, but the address check is still based on the original block number in sd->erase_{start, end}. Fixes: 1bd6fd8ed593 ("hw/sd/sdcard: Do not attempt to erase out of range addresses") Signed-off-by: Bin Meng Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210216150225.27996-2-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 172e83f99d9..7adcb4edfaa 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -760,7 +760,7 @@ static void sd_erase(SDState *sd) erase_end *= 512; } -if (sd->erase_start > sd->size || sd->erase_end > sd->size) { +if (erase_start > sd->size || erase_end > sd->size) { sd->card_status |= OUT_OF_RANGE; sd->erase_start = INVALID_ADDRESS; sd->erase_end = INVALID_ADDRESS; -- 2.26.2
[PULL 16/18] hw/sd: sd: Bypass the RCA check for CMD13 in SPI mode
From: Bin Meng Unlike SD mode, when SD card is working in SPI mode, the argument of CMD13 is stuff bits. Hence we should bypass the RCA check. See "Physical Layer Specification Version 8.00", chapter 7.3.1.3 Detailed Command Description (SPI mode): "The card shall ignore stuff bits and reserved bits in an argument" and Table 7-3 Commands and Arguments (SPI mode): "CMD13 Argument [31:0] stuff bits" Signed-off-by: Bin Meng Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210216150225.27996-9-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 3a515a5365f..8b397effbcc 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -1163,8 +1163,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) case 13: /* CMD13: SEND_STATUS */ switch (sd->mode) { case sd_data_transfer_mode: -if (sd->rca != rca) +if (!sd->spi && sd->rca != rca) { return sd_r0; +} return sd_r1; -- 2.26.2
[PULL 08/18] hw/sd: ssi-sd: Fix STOP_TRANSMISSION (CMD12) response
From: Bin Meng CMD12's response type is R1b, which is basically a R1 plus optional addition of the busy signal token that can be any number of bytes. A zero value indicates card is busy and a non-zero value indicates the card is ready for the next command. Current implementation sends the busy signal token without sending the R1 first. This does not break the U-Boot/Linux mmc_spi driver, but it does not make the VxWorks driver happy. Move the testing logic of s->stopping in the SSI_SD_RESPONSE state a bit later, after the first byte of the card reponse is sent out, to conform with the spec. After the busy signal token is sent, the state should be transferred to SSI_SD_CMD. Fixes: 775616c3ae8c ("Partial SD card SPI mode support") Signed-off-by: Bin Meng Message-Id: <20210128063035.15674-9-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/ssi-sd.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c index 84c873b3fd4..907d681d19e 100644 --- a/hw/sd/ssi-sd.c +++ b/hw/sd/ssi-sd.c @@ -243,14 +243,15 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val) s->mode = SSI_SD_RESPONSE; return SSI_DUMMY; case SSI_SD_RESPONSE: -if (s->stopping) { -s->stopping = 0; -return SSI_DUMMY; -} if (s->response_pos < s->arglen) { DPRINTF("Response 0x%02x\n", s->response[s->response_pos]); return s->response[s->response_pos++]; } +if (s->stopping) { +s->stopping = 0; +s->mode = SSI_SD_CMD; +return SSI_DUMMY; +} if (sdbus_data_ready(>sdbus)) { DPRINTF("Data read\n"); s->mode = SSI_SD_DATA_START; -- 2.26.2
[PULL 09/18] hw/sd: ssi-sd: Handle the rest commands with R1b response type
From: Bin Meng Besides CMD12, the following command's reponse type is R1b: - SET_WRITE_PROT (CMD28) - CLR_WRITE_PROT (CMD29) - ERASE (CMD38) Reuse the same s->stopping to indicate a R1b reponse is needed. Signed-off-by: Bin Meng Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210128063035.15674-10-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/ssi-sd.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c index 907d681d19e..97ee58e20cf 100644 --- a/hw/sd/ssi-sd.c +++ b/hw/sd/ssi-sd.c @@ -194,6 +194,12 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val) /* CMD13 returns a 2-byte statuse work. Other commands only return the first byte. */ s->arglen = (s->cmd == 13) ? 2 : 1; + +/* handle R1b */ +if (s->cmd == 28 || s->cmd == 29 || s->cmd == 38) { +s->stopping = 1; +} + cardstatus = ldl_be_p(longresp); status = 0; if (((cardstatus >> 9) & 0xf) < 4) -- 2.26.2
[PATCH v5 2/5] util/qemu-sockets.c: Split host:port parsing out of inet_parse
The parsing is moved into new function inet_parse_host_and_port. This is done in preparation for using the function in net/slirp.c. Signed-off-by: Doug Evans --- Changes from v4: - move recognition of "[]:port" to separate patch - allow passing NULL for ip_v6 - fix some formatting issues Changes from v3: - this patch is new in v4 - provides new utility: inet_parse_host_and_port, updates inet_parse to use it include/qemu/sockets.h | 3 ++ util/qemu-sockets.c| 80 +++--- 2 files changed, 62 insertions(+), 21 deletions(-) diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h index 7d1f813576..b1448cfa24 100644 --- a/include/qemu/sockets.h +++ b/include/qemu/sockets.h @@ -31,6 +31,9 @@ int socket_set_fast_reuse(int fd); int inet_ai_family_from_address(InetSocketAddress *addr, Error **errp); +const char *inet_parse_host_and_port(const char *str, int terminator, + char **hostp, char **portp, bool *is_v6, + Error **errp); int inet_parse(InetSocketAddress *addr, const char *str, Error **errp); int inet_connect(const char *str, Error **errp); int inet_connect_saddr(InetSocketAddress *saddr, Error **errp); diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index 8af0278f15..3ca6a6fb3d 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -615,44 +615,82 @@ static int inet_parse_flag(const char *flagname, const char *optstr, bool *val, return 0; } -int inet_parse(InetSocketAddress *addr, const char *str, Error **errp) +/* + * Parse an inet host and port as "host:port". + * Terminator may be '\0'. + * The syntax for IPv4 addresses is: address:port. "address" is optional, + * and may be empty (i.e., str is ":port"). + * The syntax for IPv6 addresses is: [address]:port. Upon return the wrapping + * [] brackets are removed. + * Host names are also supported as hostname:port. It is up to the caller to + * distinguish host names from numeric IPv4 addresses. + * On success, returns a pointer to the terminator. Space for the address and + * port is malloced and stored in *host, *port, the caller must free. + * If is_v6 is non-NULL, then it is set to true if the address is an IPv6 + * address (i.e., [address]), otherwise it is set to false. + * On failure NULL is returned with the error stored in *errp. + */ +const char *inet_parse_host_and_port(const char *str, int terminator, + char **hostp, char **portp, bool *is_v6, + Error **errp) { -const char *optstr, *h; +const char *terminator_ptr = strchr(str, terminator); +g_autofree char *buf = NULL; char host[65]; char port[33]; -int to; -int pos; -char *begin; -memset(addr, 0, sizeof(*addr)); +if (terminator_ptr == NULL) { +/* If the terminator isn't found then use the entire string. */ +terminator_ptr = str + strlen(str); +} +buf = g_strndup(str, terminator_ptr - str); -/* parse address */ -if (str[0] == ':') { +if (buf[0] == ':') { /* no host given */ host[0] = '\0'; -if (sscanf(str, ":%32[^,]%n", port, ) != 1) { -error_setg(errp, "error parsing port in address '%s'", str); -return -1; +if (sscanf(buf, ":%32s", port) != 1) { +error_setg(errp, "error parsing port in address '%s'", buf); +return NULL; } -} else if (str[0] == '[') { +} else if (buf[0] == '[') { /* IPv6 addr */ -if (sscanf(str, "[%64[^]]]:%32[^,]%n", host, port, ) != 2) { -error_setg(errp, "error parsing IPv6 address '%s'", str); -return -1; +if (sscanf(buf, "[%64[^]]]:%32s", host, port) != 2) { +error_setg(errp, "error parsing IPv6 address '%s'", buf); +return NULL; } } else { /* hostname or IPv4 addr */ -if (sscanf(str, "%64[^:]:%32[^,]%n", host, port, ) != 2) { -error_setg(errp, "error parsing address '%s'", str); -return -1; +if (sscanf(buf, "%64[^:]:%32s", host, port) != 2) { +error_setg(errp, "error parsing address '%s'", buf); +return NULL; } } -addr->host = g_strdup(host); -addr->port = g_strdup(port); +*hostp = g_strdup(host); +*portp = g_strdup(port); +if (is_v6 != NULL) { +*is_v6 = buf[0] == '['; +} + +return terminator_ptr; +} + +int inet_parse(InetSocketAddress *addr, const char *str, Error **errp) +{ +const char *optstr, *h; +int to; +int pos; +char *begin; + +memset(addr, 0, sizeof(*addr)); + +optstr = inet_parse_host_and_port(str, ',', >host, >port, + NULL, errp); +if (optstr == NULL) { +return -1; +} /* parse options */ -optstr = str + pos; h = strstr(optstr,
[PULL 15/18] hw/sd: sd: Skip write protect groups check in CMD24/25 for high capacity cards
From: Bin Meng High capacity cards don't support write protection hence we should not perform the write protect groups check in CMD24/25 for them. Signed-off-by: Bin Meng Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210216150225.27996-8-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 883c04de028..3a515a5365f 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -1268,8 +1268,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) sd->data_offset = 0; sd->blk_written = 0; -if (sd_wp_addr(sd, sd->data_start)) { -sd->card_status |= WP_VIOLATION; +if (sd->size <= SDSC_MAX_CAPACITY) { +if (sd_wp_addr(sd, sd->data_start)) { +sd->card_status |= WP_VIOLATION; +} } if (sd->csd[14] & 0x30) { sd->card_status |= WP_VIOLATION; @@ -1821,9 +1823,11 @@ void sd_write_byte(SDState *sd, uint8_t value) sd->card_status |= ADDRESS_ERROR; break; } -if (sd_wp_addr(sd, sd->data_start)) { -sd->card_status |= WP_VIOLATION; -break; +if (sd->size <= SDSC_MAX_CAPACITY) { +if (sd_wp_addr(sd, sd->data_start)) { +sd->card_status |= WP_VIOLATION; +break; +} } } sd->data[sd->data_offset++] = value; -- 2.26.2
[PULL 07/18] hw/sd: ssi-sd: Fix SEND_IF_COND (CMD8) response
From: Bin Meng The SEND_IF_COND command (CMD8) response is of format R7, but current code returns R1 for CMD8. Fix it. Fixes: 775616c3ae8c ("Partial SD card SPI mode support") Signed-off-by: Bin Meng Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210128063035.15674-8-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/ssi-sd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c index 200e885225a..84c873b3fd4 100644 --- a/hw/sd/ssi-sd.c +++ b/hw/sd/ssi-sd.c @@ -176,9 +176,9 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val) s->arglen = 1; s->response[0] = 4; DPRINTF("SD command failed\n"); -} else if (s->cmd == 58) { -/* CMD58 returns R3 response (OCR) */ -DPRINTF("Returned OCR\n"); +} else if (s->cmd == 8 || s->cmd == 58) { +/* CMD8/CMD58 returns R3/R7 response */ +DPRINTF("Returned R3/R7\n"); s->arglen = 5; s->response[0] = 1; memcpy(>response[1], longresp, 4); -- 2.26.2
[PULL 06/18] hw/sd: ssi-sd: Support multiple block write
From: Bin Meng For a multiple block write operation, each block begins with a multi write start token. Unlike the SD mode that the multiple block write ends when receiving a STOP_TRAN command (CMD12), a special stop tran token is used to signal the card. Emulating this by manually sending a CMD12 to the SD card core, to bring it out of the receiving data state. Signed-off-by: Bin Meng Tested-by: Philippe Mathieu-Daudé Acked-by: Alistair Francis Message-Id: <20210128063035.15674-7-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/ssi-sd.c | 33 +++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c index 1205ad8b52c..200e885225a 100644 --- a/hw/sd/ssi-sd.c +++ b/hw/sd/ssi-sd.c @@ -4,6 +4,11 @@ * Copyright (c) 2007-2009 CodeSourcery. * Written by Paul Brook * + * Copyright (c) 2021 Wind River Systems, Inc. + * Improved by Bin Meng + * + * Validated with U-Boot v2021.01 and Linux v5.10 mmc_spi driver + * * This code is licensed under the GNU GPL v2. * * Contributions after 2012-01-13 are licensed under the terms of the @@ -82,6 +87,10 @@ OBJECT_DECLARE_SIMPLE_TYPE(ssi_sd_state, SSI_SD) #define SSI_SDR_ADDRESS_ERROR 0x2000 #define SSI_SDR_PARAMETER_ERROR 0x4000 +/* multiple block write */ +#define SSI_TOKEN_MULTI_WRITE 0xfc +/* terminate multiple block write */ +#define SSI_TOKEN_STOP_TRAN 0xfd /* single block read/write, multiple block read */ #define SSI_TOKEN_SINGLE0xfe @@ -94,6 +103,8 @@ OBJECT_DECLARE_SIMPLE_TYPE(ssi_sd_state, SSI_SD) static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val) { ssi_sd_state *s = SSI_SD(dev); +SDRequest request; +uint8_t longresp[16]; /* * Special case: allow CMD12 (STOP TRANSMISSION) while reading data. @@ -125,8 +136,28 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val) return SSI_DUMMY; break; case SSI_TOKEN_SINGLE: +case SSI_TOKEN_MULTI_WRITE: DPRINTF("Start write block\n"); s->mode = SSI_SD_DATA_WRITE; +return SSI_DUMMY; +case SSI_TOKEN_STOP_TRAN: +DPRINTF("Stop multiple write\n"); + +/* manually issue cmd12 to stop the transfer */ +request.cmd = 12; +request.arg = 0; +s->arglen = sdbus_do_command(>sdbus, , longresp); +if (s->arglen <= 0) { +s->arglen = 1; +/* a zero value indicates the card is busy */ +s->response[0] = 0; +DPRINTF("SD card busy\n"); +} else { +s->arglen = 1; +/* a non-zero value indicates the card is ready */ +s->response[0] = SSI_DUMMY; +} + return SSI_DUMMY; } @@ -136,8 +167,6 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val) return SSI_DUMMY; case SSI_SD_CMDARG: if (s->arglen == 4) { -SDRequest request; -uint8_t longresp[16]; /* FIXME: Check CRC. */ request.cmd = s->cmd; request.arg = ldl_be_p(s->cmdarg); -- 2.26.2
[PATCH v5 4/5] net/slirp.c: Refactor address parsing
... in preparation for adding ipv6 host forwarding support. New test: avocado run tests/acceptance/hostfwd.py Signed-off-by: Doug Evans --- Changes from v4: - was 3/4 in v4 - fix some formatting issues Changes from v3: - this patch renamed from 2/3 to 3/4 - call inet_parse_host_and_port from util/qemu-sockets.c - added tests/acceptance/hostfwd.py Changes from v2: - nothing of consequence Changes from v1: - this patch is new in v2 - address parsing refactor split out, ipv4 changes here - libslirp part is now upstream in libslirp repo net/slirp.c | 165 ++-- tests/acceptance/hostfwd.py | 94 2 files changed, 196 insertions(+), 63 deletions(-) create mode 100644 tests/acceptance/hostfwd.py diff --git a/net/slirp.c b/net/slirp.c index be914c0be0..e0284492b9 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -631,15 +631,91 @@ static SlirpState *slirp_lookup(Monitor *mon, const char *id) } } +/* + * Parse a protocol name of the form "name". + * Valid protocols are "tcp" and "udp". An empty string means "tcp". + * Returns a pointer to the end of the parsed string on success, and stores + * the result in *is_udp. + * Otherwise returns NULL and stores the error in *errp. + */ +static const char *parse_protocol(const char *str, int sep, bool *is_udp, + Error **errp) +{ +char buf[10]; +const char *p = str; + +if (get_str_sep(buf, sizeof(buf), , sep) < 0) { +error_setg(errp, "Missing protocol name separator"); +return NULL; +} + +if (!strcmp(buf, "tcp") || buf[0] == '\0') { +*is_udp = false; +} else if (!strcmp(buf, "udp")) { +*is_udp = true; +} else { +error_setg(errp, "Bad protocol name"); +return NULL; +} + +return p; +} + +/* + * Parse an ip address/port of the form "address:port". + * An empty address means INADDR_ANY. + * Returns a pointer to after the terminator, unless it was '\0' in which case + * the result points to the '\0'. + * The parsed results are stored in *addr, *port. + * On error NULL is returned and stores the error in *errp. + */ +static const char *parse_ip_addr_and_port(const char *str, int terminator, + struct in_addr *addr, int *port, + Error **errp) +{ +g_autofree char *addr_str = NULL; +g_autofree char *port_str = NULL; +bool is_v6; +const char *p = inet_parse_host_and_port(str, terminator, _str, + _str, _v6, errp); + +if (p == NULL) { +return NULL; +} + +/* Ignore is_v6 for the moment, if inet_aton fails let it. */ +if (addr_str[0] == '\0') { +addr->s_addr = INADDR_ANY; +} else if (!inet_aton(addr_str, addr)) { +error_setg(errp, "Bad address"); +return NULL; +} + +if (qemu_strtoi(port_str, NULL, 10, port) < 0 || +*port < 0 || *port > 65535) { +error_setg(errp, "Bad port"); +return NULL; +} + +/* + * At this point "p" points to the terminator or trailing NUL if the + * terminator is not present. + */ +if (*p) { +++p; +} +return p; +} + void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict) { -struct in_addr host_addr = { .s_addr = INADDR_ANY }; +struct in_addr host_addr; int host_port; -char buf[256]; const char *src_str, *p; SlirpState *s; -int is_udp = 0; +bool is_udp; int err; +Error *error = NULL; const char *arg1 = qdict_get_str(qdict, "arg1"); const char *arg2 = qdict_get_try_str(qdict, "arg2"); @@ -654,30 +730,18 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict) return; } +g_assert(src_str != NULL); p = src_str; -if (!p || get_str_sep(buf, sizeof(buf), , ':') < 0) { -goto fail_syntax; -} - -if (!strcmp(buf, "tcp") || buf[0] == '\0') { -is_udp = 0; -} else if (!strcmp(buf, "udp")) { -is_udp = 1; -} else { -goto fail_syntax; -} -if (get_str_sep(buf, sizeof(buf), , ':') < 0) { -goto fail_syntax; -} -if (buf[0] != '\0' && !inet_aton(buf, _addr)) { +p = parse_protocol(p, ':', _udp, ); +if (p == NULL) { goto fail_syntax; } -if (qemu_strtoi(p, NULL, 10, _port)) { +if (parse_ip_addr_and_port(p, '\0', _addr, _port, + ) == NULL) { goto fail_syntax; } - err = slirp_remove_hostfwd(s->slirp, is_udp, host_addr, host_port); monitor_printf(mon, "host forwarding rule for %s %s\n", src_str, @@ -685,65 +749,39 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict) return; fail_syntax: -monitor_printf(mon, "invalid format\n"); +monitor_printf(mon, "Invalid format: %s\n", error_get_pretty(error)); +error_free(error); } static int
[PATCH v5 0/5] Add support for ipv6 host forwarding
This patchset takes the original patch from Maxim, https://www.mail-archive.com/qemu-devel@nongnu.org/msg569573.html and updates it. Option hostfwd is extended to support ipv6 addresses. Commands hostfwd_add, hostfwd_remove are extended as well. The libslirp part of the patch has been committed upstream, and is now in qemu. See patch 1/5. Changes from v4: 1/5 slirp: Advance libslirp submodule to add ipv6 host-forward support NOTE TO REVIEWERS: I need some hand-holding to know what The Right way to submit this particular patch is. - no change 2/5 util/qemu-sockets.c: Split host:port parsing out of inet_parse - move recognition of "[]:port" to separate patch - allow passing NULL for ip_v6 - fix some formatting issues 3/5 inet_parse_host_and_addr: Recognize []:port (empty ipv6 address) - new in this patchset revision 4/5 net/slirp.c: Refactor address parsing - was 3/4 in v4 - fix some formatting issues 5/5 net: Extend host forwarding to support IPv6 - was 4/4 in v4 - fix some formatting issues Changes from v3: 1/4 slirp: Advance libslirp submodule to add ipv6 host-forward support - pick up latest libslirp patch to reject ipv6 addr-any for guest address - libslirp currently only provides a stateless DHCPv6 server, which means it can't know in advance what the guest's IP address is, and thus cannot do the "addr-any -> guest ip address" translation that is done for ipv4 2/4 util/qemu-sockets.c: Split host:port parsing out of inet_parse - this patch is new in v4 - provides new utility: inet_parse_host_and_port, updates inet_parse to use it 3/4 net/slirp.c: Refactor address parsing - this patch renamed from 2/3 to 3/4 - call inet_parse_host_and_port from util/qemu-sockets.c - added tests/acceptance/hostfwd.py 4/4 net: Extend host forwarding to support IPv6 - this patch renamed from 3/3 to 4/4 - ipv6 support added to existing hostfwd option, commands - instead of creating new ipv6 option, commands - added tests to tests/acceptance/hostfwd.py Changes from v2: - split out libslirp commit - clarify spelling of ipv6 addresses in docs - tighten parsing of ipv6 addresses Change from v1: - libslirp part is now upstream - net/slirp.c changes split into two pieces (refactor, add ipv6) - added docs Doug Evans (5): slirp: Advance libslirp submodule to add ipv6 host-forward support util/qemu-sockets.c: Split host:port parsing out of inet_parse inet_parse_host_and_addr: Recognize []:port (empty ipv6 address) net/slirp.c: Refactor address parsing net: Extend host forwarding to support IPv6 hmp-commands.hx | 15 +++ include/qemu/sockets.h | 3 + net/slirp.c | 196 slirp | 2 +- tests/acceptance/hostfwd.py | 174 util/qemu-sockets.c | 84 6 files changed, 390 insertions(+), 84 deletions(-) create mode 100644 tests/acceptance/hostfwd.py -- 2.30.0.617.g56c4b15f3c-goog
[PATCH v5 3/5] inet_parse_host_and_addr: Recognize []:port (empty ipv6 address)
Some callers need to distinguish empty ipv4 addresses from ipv6. Signed-off-by: Doug Evans --- Changes from v4: - new in this patchset revision util/qemu-sockets.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index 3ca6a6fb3d..062f0eb074 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -620,7 +620,8 @@ static int inet_parse_flag(const char *flagname, const char *optstr, bool *val, * Terminator may be '\0'. * The syntax for IPv4 addresses is: address:port. "address" is optional, * and may be empty (i.e., str is ":port"). - * The syntax for IPv6 addresses is: [address]:port. Upon return the wrapping + * The syntax for IPv6 addresses is: [address]:port. "address" is optional, + * and may be empty (i.e., str is "[]:port"). Upon return the wrapping * [] brackets are removed. * Host names are also supported as hostname:port. It is up to the caller to * distinguish host names from numeric IPv4 addresses. @@ -654,7 +655,10 @@ const char *inet_parse_host_and_port(const char *str, int terminator, } } else if (buf[0] == '[') { /* IPv6 addr */ -if (sscanf(buf, "[%64[^]]]:%32s", host, port) != 2) { +/* Note: sscanf %[ doesn't recognize empty contents. */ +if (sscanf(buf, "[]:%32s", port) == 1) { +host[0] = '\0'; +} else if (sscanf(buf, "[%64[^]]]:%32s", host, port) != 2) { error_setg(errp, "error parsing IPv6 address '%s'", buf); return NULL; } -- 2.30.0.617.g56c4b15f3c-goog
[PATCH v5 5/5] net: Extend host forwarding to support IPv6
Net option "-hostfwd" now supports IPv6 addresses. Commands hostfwd_add, hostfwd_remove now support IPv6 addresses. Signed-off-by: Doug Evans --- Changes from v4: - was 4/4 in v4 - fix some formatting issues Differences from v3: - this patch renamed from 3/3 to 4/4 - ipv6 support added to existing hostfwd option, commands - instead of creating new ipv6 option, commands - added tests to tests/acceptance/hostfwd.py Differences from v2: - clarify spelling of ipv6 addresses in docs - tighten parsing of ipv6 addresses Differences from v1: - parsing refactor split out into separate patch (2/3) hmp-commands.hx | 15 +++ net/slirp.c | 77 +-- tests/acceptance/hostfwd.py | 80 + 3 files changed, 150 insertions(+), 22 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index d4001f9c5d..4de4e4979d 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1375,6 +1375,16 @@ ERST SRST ``hostfwd_add`` Redirect TCP or UDP connections from host to guest (requires -net user). + IPV6 addresses are wrapped in square brackes, IPV4 addresses are not. + + Examples: + hostfwd_add net0 tcp:127.0.0.1:10022-:22 + hostfwd_add net0 tcp:[::1]:10022-[fe80::1:2:3:4]:22 + + Note that Libslirp currently only provides a "stateless" DHCPv6 server, a + consequence of which is that it cannot do the "addr-any" translation to the + guest address that is done for IPv4. In other words, the following is + currently not supported: hostfwd_add net0 tcp:[::1]:10022-:22 ERST #ifdef CONFIG_SLIRP @@ -1390,6 +1400,11 @@ ERST SRST ``hostfwd_remove`` Remove host-to-guest TCP or UDP redirection. + IPV6 addresses are wrapped in square brackes, IPV4 addresses are not. + + Examples: + hostfwd_remove net0 tcp:127.0.0.1:10022 + hostfwd_remove net0 tcp:[::1]:10022 ERST { diff --git a/net/slirp.c b/net/slirp.c index e0284492b9..32df65c1f0 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -96,6 +96,11 @@ typedef struct SlirpState { GSList *fwd; } SlirpState; +union in4or6_addr { +struct in_addr addr4; +struct in6_addr addr6; +}; + static struct slirp_config_str *slirp_configs; static QTAILQ_HEAD(, SlirpState) slirp_stacks = QTAILQ_HEAD_INITIALIZER(slirp_stacks); @@ -663,32 +668,40 @@ static const char *parse_protocol(const char *str, int sep, bool *is_udp, /* * Parse an ip address/port of the form "address:port". - * An empty address means INADDR_ANY. + * IPv6 addresses are wrapped in [] brackets. + * An empty address means INADDR_ANY/in6addr_any. * Returns a pointer to after the terminator, unless it was '\0' in which case * the result points to the '\0'. - * The parsed results are stored in *addr, *port. + * The parsed results are stored in *addr, *port, *is_v6. * On error NULL is returned and stores the error in *errp. */ static const char *parse_ip_addr_and_port(const char *str, int terminator, - struct in_addr *addr, int *port, - Error **errp) + union in4or6_addr *addr, int *port, + bool *is_v6, Error **errp) { g_autofree char *addr_str = NULL; g_autofree char *port_str = NULL; -bool is_v6; const char *p = inet_parse_host_and_port(str, terminator, _str, - _str, _v6, errp); + _str, is_v6, errp); if (p == NULL) { return NULL; } -/* Ignore is_v6 for the moment, if inet_aton fails let it. */ -if (addr_str[0] == '\0') { -addr->s_addr = INADDR_ANY; -} else if (!inet_aton(addr_str, addr)) { -error_setg(errp, "Bad address"); -return NULL; +if (*is_v6) { +if (addr_str[0] == '\0') { +addr->addr6 = in6addr_any; +} else if (!inet_pton(AF_INET6, addr_str, >addr6)) { +error_setg(errp, "Bad address"); +return NULL; +} +} else { +if (addr_str[0] == '\0') { +addr->addr4.s_addr = INADDR_ANY; +} else if (!inet_pton(AF_INET, addr_str, >addr4)) { +error_setg(errp, "Bad address"); +return NULL; +} } if (qemu_strtoi(port_str, NULL, 10, port) < 0 || @@ -709,11 +722,11 @@ static const char *parse_ip_addr_and_port(const char *str, int terminator, void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict) { -struct in_addr host_addr; +union in4or6_addr host_addr; int host_port; const char *src_str, *p; SlirpState *s; -bool is_udp; +bool is_udp, is_v6; int err; Error *error = NULL; const char *arg1 = qdict_get_str(qdict, "arg1"); @@ -738,11 +751,18 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict) goto fail_syntax; } -if (parse_ip_addr_and_port(p, '\0', _addr,
[PULL 03/18] hw/sd: sd: Allow single/multiple block write for SPI mode
From: Bin Meng At present the single/multiple block write in SPI mode is blocked by sd_normal_command(). Remove the limitation. Signed-off-by: Bin Meng Tested-by: Philippe Mathieu-Daudé Acked-by: Alistair Francis Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210128063035.15674-4-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index a85a821abbe..5de9e0a6c20 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -1230,9 +1230,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) case 25: /* CMD25: WRITE_MULTIPLE_BLOCK */ switch (sd->state) { case sd_transfer_state: -/* Writing in SPI mode not implemented. */ -if (sd->spi) -break; if (addr + sd->blk_len > sd->size) { sd->card_status |= ADDRESS_ERROR; -- 2.26.2
[PULL 04/18] hw/sd: Introduce receive_ready() callback
From: Bin Meng At present there is a data_ready() callback for the SD data read path. Let's add a receive_ready() for the SD data write path. Signed-off-by: Bin Meng Tested-by: Philippe Mathieu-Daudé Reviewed-by: Philippe Mathieu-Daudé Acked-by: Alistair Francis Message-Id: <20210128063035.15674-5-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- include/hw/sd/sd.h | 2 ++ hw/sd/core.c | 13 + hw/sd/sd.c | 6 ++ 3 files changed, 21 insertions(+) diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h index 05ef9b73e56..47360ba4ee9 100644 --- a/include/hw/sd/sd.h +++ b/include/hw/sd/sd.h @@ -116,6 +116,7 @@ struct SDCardClass { * Return: byte value read */ uint8_t (*read_byte)(SDState *sd); +bool (*receive_ready)(SDState *sd); bool (*data_ready)(SDState *sd); void (*set_voltage)(SDState *sd, uint16_t millivolts); uint8_t (*get_dat_lines)(SDState *sd); @@ -187,6 +188,7 @@ void sdbus_write_data(SDBus *sdbus, const void *buf, size_t length); * Read multiple bytes of data on the data lines of a SD bus. */ void sdbus_read_data(SDBus *sdbus, void *buf, size_t length); +bool sdbus_receive_ready(SDBus *sd); bool sdbus_data_ready(SDBus *sd); bool sdbus_get_inserted(SDBus *sd); bool sdbus_get_readonly(SDBus *sd); diff --git a/hw/sd/core.c b/hw/sd/core.c index 08c93b59034..30ee62c5106 100644 --- a/hw/sd/core.c +++ b/hw/sd/core.c @@ -160,6 +160,19 @@ void sdbus_read_data(SDBus *sdbus, void *buf, size_t length) } } +bool sdbus_receive_ready(SDBus *sdbus) +{ +SDState *card = get_card(sdbus); + +if (card) { +SDCardClass *sc = SD_CARD_GET_CLASS(card); + +return sc->receive_ready(card); +} + +return false; +} + bool sdbus_data_ready(SDBus *sdbus) { SDState *card = get_card(sdbus); diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 5de9e0a6c20..172e83f99d9 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -2037,6 +2037,11 @@ uint8_t sd_read_byte(SDState *sd) return ret; } +static bool sd_receive_ready(SDState *sd) +{ +return sd->state == sd_receivingdata_state; +} + static bool sd_data_ready(SDState *sd) { return sd->state == sd_sendingdata_state; @@ -2147,6 +2152,7 @@ static void sd_class_init(ObjectClass *klass, void *data) sc->do_command = sd_do_command; sc->write_byte = sd_write_byte; sc->read_byte = sd_read_byte; +sc->receive_ready = sd_receive_ready; sc->data_ready = sd_data_ready; sc->enable = sd_enable; sc->get_inserted = sd_get_inserted; -- 2.26.2
[PULL 01/18] hw/sd: ssi-sd: Support multiple block read
From: Bin Meng In the case of a multiple block read operation every transferred block has its suffix of CRC16. Update the state machine logic to handle multiple block read. Signed-off-by: Bin Meng [PMD: Change VMState version id 5 -> 6] Signed-off-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé Acked-by: Alistair Francis Message-Id: <20210128063035.15674-2-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/ssi-sd.c | 42 +- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c index be1bb101645..6d20a240c69 100644 --- a/hw/sd/ssi-sd.c +++ b/hw/sd/ssi-sd.c @@ -52,6 +52,7 @@ struct ssi_sd_state { uint8_t cmdarg[4]; uint8_t response[5]; uint16_t crc16; +int32_t read_bytes; int32_t arglen; int32_t response_pos; int32_t stopping; @@ -88,11 +89,26 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val) { ssi_sd_state *s = SSI_SD(dev); -/* Special case: allow CMD12 (STOP TRANSMISSION) while reading data. */ -if (s->mode == SSI_SD_DATA_READ && val == 0x4c) { -s->mode = SSI_SD_CMD; -/* There must be at least one byte delay before the card responds. */ -s->stopping = 1; +/* + * Special case: allow CMD12 (STOP TRANSMISSION) while reading data. + * + * See "Physical Layer Specification Version 8.00" chapter 7.5.2.2, + * to avoid conflict between CMD12 response and next data block, + * timing of CMD12 should be controlled as follows: + * + * - CMD12 issued at the timing that end bit of CMD12 and end bit of + * data block is overlapped + * - CMD12 issued after one clock cycle after host receives a token + * (either Start Block token or Data Error token) + * + * We need to catch CMD12 in all of the data read states. + */ +if (s->mode >= SSI_SD_PREP_DATA && s->mode <= SSI_SD_DATA_CRC16) { +if (val == 0x4c) { +s->mode = SSI_SD_CMD; +/* There must be at least one byte delay before the card responds */ +s->stopping = 1; +} } switch (s->mode) { @@ -212,8 +228,9 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val) return SSI_TOKEN_SINGLE; case SSI_SD_DATA_READ: val = sdbus_read_byte(>sdbus); +s->read_bytes++; s->crc16 = crc_ccitt_false(s->crc16, (uint8_t *), 1); -if (!sdbus_data_ready(>sdbus)) { +if (!sdbus_data_ready(>sdbus) || s->read_bytes == 512) { DPRINTF("Data read end\n"); s->mode = SSI_SD_DATA_CRC16; } @@ -224,7 +241,12 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val) s->response_pos++; if (s->response_pos == 2) { DPRINTF("CRC16 read end\n"); -s->mode = SSI_SD_CMD; +if (s->read_bytes == 512 && s->cmd != 17) { +s->mode = SSI_SD_PREP_DATA; +} else { +s->mode = SSI_SD_CMD; +} +s->read_bytes = 0; s->response_pos = 0; } return val; @@ -255,8 +277,8 @@ static int ssi_sd_post_load(void *opaque, int version_id) static const VMStateDescription vmstate_ssi_sd = { .name = "ssi_sd", -.version_id = 5, -.minimum_version_id = 5, +.version_id = 6, +.minimum_version_id = 6, .post_load = ssi_sd_post_load, .fields = (VMStateField []) { VMSTATE_UINT32(mode, ssi_sd_state), @@ -264,6 +286,7 @@ static const VMStateDescription vmstate_ssi_sd = { VMSTATE_UINT8_ARRAY(cmdarg, ssi_sd_state, 4), VMSTATE_UINT8_ARRAY(response, ssi_sd_state, 5), VMSTATE_UINT16(crc16, ssi_sd_state), +VMSTATE_INT32(read_bytes, ssi_sd_state), VMSTATE_INT32(arglen, ssi_sd_state), VMSTATE_INT32(response_pos, ssi_sd_state), VMSTATE_INT32(stopping, ssi_sd_state), @@ -316,6 +339,7 @@ static void ssi_sd_reset(DeviceState *dev) memset(s->cmdarg, 0, sizeof(s->cmdarg)); memset(s->response, 0, sizeof(s->response)); s->crc16 = 0; +s->read_bytes = 0; s->arglen = 0; s->response_pos = 0; s->stopping = 0; -- 2.26.2
[PULL 05/18] hw/sd: ssi-sd: Support single block write
From: Bin Meng Add 2 more states for the block write operation. The SPI host needs to send a data start token to start the transfer, and the data block written to the card will be acknowledged by a data response token. Signed-off-by: Bin Meng [PMD: Change VMState version id 6 -> 7] Signed-off-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé Acked-by: Alistair Francis Message-Id: <20210128063035.15674-6-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/ssi-sd.c | 44 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c index 6d20a240c69..1205ad8b52c 100644 --- a/hw/sd/ssi-sd.c +++ b/hw/sd/ssi-sd.c @@ -43,6 +43,8 @@ typedef enum { SSI_SD_DATA_START, SSI_SD_DATA_READ, SSI_SD_DATA_CRC16, +SSI_SD_DATA_WRITE, +SSI_SD_SKIP_CRC16, } ssi_sd_mode; struct ssi_sd_state { @@ -53,6 +55,7 @@ struct ssi_sd_state { uint8_t response[5]; uint16_t crc16; int32_t read_bytes; +int32_t write_bytes; int32_t arglen; int32_t response_pos; int32_t stopping; @@ -85,6 +88,9 @@ OBJECT_DECLARE_SIMPLE_TYPE(ssi_sd_state, SSI_SD) /* dummy value - don't care */ #define SSI_DUMMY 0xff +/* data accepted */ +#define DATA_RESPONSE_ACCEPTED 0x05 + static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val) { ssi_sd_state *s = SSI_SD(dev); @@ -113,10 +119,17 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val) switch (s->mode) { case SSI_SD_CMD: -if (val == SSI_DUMMY) { +switch (val) { +case SSI_DUMMY: DPRINTF("NULL command\n"); return SSI_DUMMY; +break; +case SSI_TOKEN_SINGLE: +DPRINTF("Start write block\n"); +s->mode = SSI_SD_DATA_WRITE; +return SSI_DUMMY; } + s->cmd = val & 0x3f; s->mode = SSI_SD_CMDARG; s->arglen = 0; @@ -250,6 +263,27 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val) s->response_pos = 0; } return val; +case SSI_SD_DATA_WRITE: +sdbus_write_byte(>sdbus, val); +s->write_bytes++; +if (!sdbus_receive_ready(>sdbus) || s->write_bytes == 512) { +DPRINTF("Data write end\n"); +s->mode = SSI_SD_SKIP_CRC16; +s->response_pos = 0; +} +return val; +case SSI_SD_SKIP_CRC16: +/* we don't verify the crc16 */ +s->response_pos++; +if (s->response_pos == 2) { +DPRINTF("CRC16 receive end\n"); +s->mode = SSI_SD_RESPONSE; +s->write_bytes = 0; +s->arglen = 1; +s->response[0] = DATA_RESPONSE_ACCEPTED; +s->response_pos = 0; +} +return SSI_DUMMY; } /* Should never happen. */ return SSI_DUMMY; @@ -259,7 +293,7 @@ static int ssi_sd_post_load(void *opaque, int version_id) { ssi_sd_state *s = (ssi_sd_state *)opaque; -if (s->mode > SSI_SD_DATA_CRC16) { +if (s->mode > SSI_SD_SKIP_CRC16) { return -EINVAL; } if (s->mode == SSI_SD_CMDARG && @@ -277,8 +311,8 @@ static int ssi_sd_post_load(void *opaque, int version_id) static const VMStateDescription vmstate_ssi_sd = { .name = "ssi_sd", -.version_id = 6, -.minimum_version_id = 6, +.version_id = 7, +.minimum_version_id = 7, .post_load = ssi_sd_post_load, .fields = (VMStateField []) { VMSTATE_UINT32(mode, ssi_sd_state), @@ -287,6 +321,7 @@ static const VMStateDescription vmstate_ssi_sd = { VMSTATE_UINT8_ARRAY(response, ssi_sd_state, 5), VMSTATE_UINT16(crc16, ssi_sd_state), VMSTATE_INT32(read_bytes, ssi_sd_state), +VMSTATE_INT32(write_bytes, ssi_sd_state), VMSTATE_INT32(arglen, ssi_sd_state), VMSTATE_INT32(response_pos, ssi_sd_state), VMSTATE_INT32(stopping, ssi_sd_state), @@ -340,6 +375,7 @@ static void ssi_sd_reset(DeviceState *dev) memset(s->response, 0, sizeof(s->response)); s->crc16 = 0; s->read_bytes = 0; +s->write_bytes = 0; s->arglen = 0; s->response_pos = 0; s->stopping = 0; -- 2.26.2
[PATCH v5 1/5] slirp: Advance libslirp submodule to add ipv6 host-forward support
Signed-off-by: Doug Evans --- Changes from v4: NOTE TO REVIEWERS: I need some hand-holding to know what The Right way to submit this particular patch is. - no change Changes from v3: - pick up latest libslirp patch to reject ipv6 addr-any for guest address - libslirp currently only provides a stateless DHCPv6 server, which means it can't know in advance what the guest's IP address is, and thus cannot do the "addr-any -> guest ip address" translation that is done for ipv4 Changes from v2: - this patch is new in v3, split out from v2 slirp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slirp b/slirp index 8f43a99191..26ae658a83 16 --- a/slirp +++ b/slirp @@ -1 +1 @@ -Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece +Subproject commit 26ae658a83eeca16780cf5615c8247cbb151c3fa -- 2.30.0.617.g56c4b15f3c-goog
[PULL 02/18] hw/sd: sd: Remove duplicated codes in single/multiple block read/write
From: Bin Meng The single block read (CMD17) codes are the same as the multiple block read (CMD18). Merge them into one. The same applies to single block write (CMD24) and multiple block write (CMD25). Signed-off-by: Bin Meng Tested-by: Philippe Mathieu-Daudé Acked-by: Alistair Francis Message-Id: <20210128063035.15674-3-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 47 --- 1 file changed, 47 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 8517dbce8ba..a85a821abbe 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -1181,24 +1181,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) break; case 17: /* CMD17: READ_SINGLE_BLOCK */ -switch (sd->state) { -case sd_transfer_state: - -if (addr + sd->blk_len > sd->size) { -sd->card_status |= ADDRESS_ERROR; -return sd_r1; -} - -sd->state = sd_sendingdata_state; -sd->data_start = addr; -sd->data_offset = 0; -return sd_r1; - -default: -break; -} -break; - case 18: /* CMD18: READ_MULTIPLE_BLOCK */ switch (sd->state) { case sd_transfer_state: @@ -1245,35 +1227,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) /* Block write commands (Class 4) */ case 24: /* CMD24: WRITE_SINGLE_BLOCK */ -switch (sd->state) { -case sd_transfer_state: -/* Writing in SPI mode not implemented. */ -if (sd->spi) -break; - -if (addr + sd->blk_len > sd->size) { -sd->card_status |= ADDRESS_ERROR; -return sd_r1; -} - -sd->state = sd_receivingdata_state; -sd->data_start = addr; -sd->data_offset = 0; -sd->blk_written = 0; - -if (sd_wp_addr(sd, sd->data_start)) { -sd->card_status |= WP_VIOLATION; -} -if (sd->csd[14] & 0x30) { -sd->card_status |= WP_VIOLATION; -} -return sd_r1; - -default: -break; -} -break; - case 25: /* CMD25: WRITE_MULTIPLE_BLOCK */ switch (sd->state) { case sd_transfer_state: -- 2.26.2
[PULL 00/18] SD/MMC patches for 2021-02-20
The following changes since commit e90ef02389dc8b57eaea22b290244609d720a8bf: Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2021-02-18' into staging (2021-02-19 17:22:42 +) are available in the Git repository at: https://gitlab.com/philmd/qemu.git tags/sdmmc-20210220 for you to fetch changes up to 3e0a7693be30d6a6eda8a56f3862ac2e502a9e81: MAINTAINERS: Add Bin Meng as co-maintainer for SD/MMC cards (2021-02-20 01:08:59 +0100) SD/MMC patches - Various improvements for SD cards in SPI mode (Bin Meng) - Add Bin Meng as SD/MMC cards co-maintainer Bin Meng (17): hw/sd: ssi-sd: Support multiple block read hw/sd: sd: Remove duplicated codes in single/multiple block read/write hw/sd: sd: Allow single/multiple block write for SPI mode hw/sd: Introduce receive_ready() callback hw/sd: ssi-sd: Support single block write hw/sd: ssi-sd: Support multiple block write hw/sd: ssi-sd: Fix SEND_IF_COND (CMD8) response hw/sd: ssi-sd: Fix STOP_TRANSMISSION (CMD12) response hw/sd: ssi-sd: Handle the rest commands with R1b response type hw/sd: sd: Fix address check in sd_erase() hw/sd: sd: Only SDSC cards support CMD28/29/30 hw/sd: sd: Fix CMD30 response type hw/sd: sd: Move the sd_block_{read, write} and macros ahead hw/sd: sd: Skip write protect groups check in sd_erase() for high capacity cards hw/sd: sd: Skip write protect groups check in CMD24/25 for high capacity cards hw/sd: sd: Bypass the RCA check for CMD13 in SPI mode hw/sd: sdhci: Simplify updating s->prnsts in sdhci_sdma_transfer_multi_blocks() Philippe Mathieu-Daudé (1): MAINTAINERS: Add Bin Meng as co-maintainer for SD/MMC cards include/hw/sd/sd.h | 2 + hw/sd/core.c | 13 hw/sd/sd.c | 149 +++-- hw/sd/sdhci.c | 7 +-- hw/sd/ssi-sd.c | 136 +++-- MAINTAINERS| 1 + 6 files changed, 199 insertions(+), 109 deletions(-) -- 2.26.2
Re: [PATCH] MAINTAINERS: Add Bin Meng as co-maintainer for SD/MMC cards
On 2/16/21 2:28 PM, Philippe Mathieu-Daudé wrote: > There is new interest in the SD/MMC device emulation, so it > would be good to have more than only one maintainer / reviewer > for it. > > Bin Meng proved by his contributions a deep understanding of the > SD cards internals, so let's add him to the corresponding section > in the MAINTAINERS file. > > Signed-off-by: Philippe Mathieu-Daudé > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) Thanks, applied to sdmmc-next tree.
Re: [PATCH v2 4/6] hw/sd: sdhci: Simplify updating s->prnsts in sdhci_sdma_transfer_multi_blocks()
On 2/16/21 4:46 AM, Bin Meng wrote: > s->prnsts is updated in both branches of the if () else () statement. > Move the common bits outside so that it is cleaner. > > Signed-off-by: Bin Meng > --- > > (no changes since v1) > > hw/sd/sdhci.c | 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) As there are some questions in this series and it makes sense to merge all patches at once to help downstream distributions track the CVE fixes, I'm queuing this single patch to sdmmc-next tree. Thanks, Phil.
Re: [PATCH 2/2] hw/timer/renesas_tmr: Fix use of uninitialized data in read_tcnt()
On 2/19/21 11:32 PM, Peter Maydell wrote: > The read_tcnt() function calculates the TCNT register values for the > two channels of the timer module; it sets these up in the local > tcnt[] array, and eventually returns either one or both of them, > depending on whether the access is 8 or 16 bits. However, not all of > the code paths through this function set both elements of this array: > if the guest has programmed the TCCR.CSS register fields to values > which are either documented as not to be used or which QEMU does not > implement, then the function will return uninitialized data. (This > was spotted by Coverity.) > > Add the missing CSS cases to this code, so that we return a > consistent value instead of uninitialized data, and so the code > structure indicates what's happening. > > Fixes: CID 1429976 > Signed-off-by: Peter Maydell > --- > hw/timer/renesas_tmr.c | 19 +++ > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/hw/timer/renesas_tmr.c b/hw/timer/renesas_tmr.c > index 22260aaaba5..eed39917fec 100644 > --- a/hw/timer/renesas_tmr.c > +++ b/hw/timer/renesas_tmr.c > @@ -46,7 +46,9 @@ REG8(TCCR, 10) >FIELD(TCCR, CSS, 3, 2) >FIELD(TCCR, TMRIS, 7, 1) > > +#define CSS_EXTERNAL 0x00 > #define CSS_INTERNAL 0x01 > +#define CSS_INVALID 0x02 > #define CSS_CASCADING 0x03 > #define CCLR_A0x01 > #define CCLR_B0x02 > @@ -130,13 +132,20 @@ static uint16_t read_tcnt(RTMRState *tmr, unsigned > size, int ch) > if (delta > 0) { > tmr->tick = now; > > -if (FIELD_EX8(tmr->tccr[1], TCCR, CSS) == CSS_INTERNAL) { > +switch (FIELD_EX8(tmr->tccr[1], TCCR, CSS)) { > +case CSS_INTERNAL: > /* timer1 count update */ > elapsed = elapsed_time(tmr, 1, delta); > if (elapsed >= 0x100) { > ovf = elapsed >> 8; > } > tcnt[1] = tmr->tcnt[1] + (elapsed & 0xff); > +break; > +case CSS_INVALID: /* guest error to have set this */ > +case CSS_EXTERNAL: /* QEMU doesn't implement these */ > +case CSS_CASCADING: > +tcnt[1] = tmr->tcnt[1]; > +break; > } > switch (FIELD_EX8(tmr->tccr[0], TCCR, CSS)) { > case CSS_INTERNAL: > @@ -144,9 +153,11 @@ static uint16_t read_tcnt(RTMRState *tmr, unsigned size, > int ch) > tcnt[0] = tmr->tcnt[0] + elapsed; > break; > case CSS_CASCADING: > -if (ovf > 0) { > -tcnt[0] = tmr->tcnt[0] + ovf; > -} > +tcnt[0] = tmr->tcnt[0] + ovf; > +break; > +case CSS_INVALID: /* guest error to have set this */ > +case CSS_EXTERNAL: /* QEMU doesn't implement this */ > +tcnt[0] = tmr->tcnt[0]; > break; > } Elegant nice fix :) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 1/2] hw/timer/renesas_tmr: Prefix constants for CSS values with CSS_
On 2/19/21 11:32 PM, Peter Maydell wrote: > The #defines INTERNAL and CASCADING represent different possible > values for the TCCR.CSS register field; prefix them with CSS_ to make > this more obvious, before we add more defines to represent the > other possible values of the field in the next commit. > > Signed-off-by: Peter Maydell > --- > hw/timer/renesas_tmr.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] hw/intc/loongson_liointc: Fix per core ISR handling
On 2/19/21 11:35 PM, Peter Maydell wrote: > This patch has been reviewed and fixes a Coverity issue; > Philippe, are you planning to take it through your MIPS tree? Sorry felt through the crack, now applied to mips-next (I'll send a pull request next week). Thanks! > > -- PMM > > On Tue, 12 Jan 2021 at 01:28, Jiaxun Yang wrote: >> >> Per core ISR is a set of 32-bit registers spaced by 8 bytes. >> This patch fixed calculation of it's size and also added check >> of alignment at reading & writing. >> >> Signed-off-by: Jiaxun Yang >> --- >> hw/intc/loongson_liointc.c | 16 +--- >> 1 file changed, 13 insertions(+), 3 deletions(-) >> >> diff --git a/hw/intc/loongson_liointc.c b/hw/intc/loongson_liointc.c >> index f823d484e0..cc11b544cb 100644 >> --- a/hw/intc/loongson_liointc.c >> +++ b/hw/intc/loongson_liointc.c >> @@ -41,7 +41,7 @@ >> #define R_IEN_CLR 0x2c >> #define R_ISR_SIZE 0x8 >> #define R_START 0x40 >> -#define R_END 0x64 >> +#define R_END (R_START + R_ISR_SIZE * NUM_CORES) >> >> struct loongson_liointc { >> SysBusDevice parent_obj; >> @@ -125,7 +125,12 @@ liointc_read(void *opaque, hwaddr addr, unsigned int >> size) >> } >> >> if (addr >= R_START && addr < R_END) { >> -int core = (addr - R_START) / R_ISR_SIZE; >> +hwaddr offset = addr - R_START; >> +int core = offset / R_ISR_SIZE; >> + >> +if (offset % R_ISR_SIZE) { >> +goto out; >> +} >> r = p->per_core_isr[core]; >> goto out; >> } >> @@ -169,7 +174,12 @@ liointc_write(void *opaque, hwaddr addr, >> } >> >> if (addr >= R_START && addr < R_END) { >> -int core = (addr - R_START) / R_ISR_SIZE; >> +hwaddr offset = addr - R_START; >> +int core = offset / R_ISR_SIZE; >> + >> +if (offset % R_ISR_SIZE) { >> +goto out; >> +} >> p->per_core_isr[core] = value; >> goto out; >> } >> -- >> 2.30.0 >> >> >
Re: problema compilation
Cc'ing Stefan / Yonggang / Paolo. On 2/20/21 12:03 AM, Peter Maydell wrote: > On Fri, 19 Feb 2021 at 22:54, nerus wrote: >> >> Good evening, I turn to you because I have a problem that does not appear in >> the official documentation, nor in the different blogs or irc channels. >> >> I need to do a cross compilation but it is impossible from version 5.2, when >> I use msys2 an error occurs indicating that symbolic links cannot be created >> even though the windows user has permissions to create symbolic links, I >> configured this through gpedit.msc. >> >> when I use cygwin with the mingw64-w64 tool chain an error occurs whereby >> meson says that it cannot find any compiler even though the compiler path is >> specified in the configuration script, mingw cannot be used from linux >> either due to There are many missing components that cannot be compiled by >> hand because the proper versions are no longer available, how could you >> solve these problems without using already compiled binaries? Thank you > > Cross compilation works in general -- our CI testing setup > includes various cross-compile configurations, including > building Windows executables from a Linux host > (eg https://gitlab.com/qemu-project/qemu/-/jobs/1042844159). > > You'll need to be more specific about exactly what you're > trying to do and failing (eg quoting exact commands, > setups, error messages). > > thanks > -- PMM >
Re: [PATCH v13 0/5] UFFD write-tracking migration/snapshots
On Fri, Feb 19, 2021 at 10:20:42PM +0100, David Hildenbrand wrote: > > A shiver just went down my spine. Please don‘t just for the sake of > > creating a snapshot. > > > > (Just imagine you don‘t have a shared zeropage...) > > ... and I just remembered we read all memory either way. Gah. > > I have some patches to make snapshots fly with virtio-mem so exactly that > won‘t happen. But they depend on vfio support, so it might take a while. Sorry I can't really follow. It'll be great if virtio-mem won't have similar problem with live snapshot finally. Is that idea applicable to balloon too, then? -- Peter Xu
Re: [PATCH v2 00/12] block/export: vhost-user-blk server tests and input validation
On Mon, 15 Feb 2021 at 10:41, Kevin Wolf wrote: > > Am 07.12.2020 um 18:20 hat Stefan Hajnoczi geschrieben: > > v2: > > * Add abrt handler that terminates qemu-storage-daemon to > >vhost-user-blk-test. No more orphaned processes on test failure. [Peter] > > * Fix sector number calculation in vhost-user-blk-server.c > > * Introduce VIRTIO_BLK_SECTOR_BITS/SIZE to make code clearer [Max] > > * Fix vhost-user-blk-server.c blk_size double byteswap > > * Fix vhost-user-blk blkcfg->num_queues endianness [Peter] > > * Squashed cleanups into Coiby vhost-user-blk-test commit so the code is > >easier to review > > > > The vhost-user-blk server test was already in Michael Tsirkin's recent vhost > > pull request, but was dropped because it exposed vhost-user regressions > > (b7c1bd9d7848 and the Based-on tag below). Now that the vhost-user > > regressions > > are fixed we can re-introduce the test case. > > > > This series adds missing input validation that led to a Coverity report. The > > virtio-blk read, write, discard, and write zeroes commands need to check > > sector/byte ranges and other inputs. This solves the issue Peter Maydell > > raised > > in "[PATCH for-5.2] block/export/vhost-user-blk-server.c: Avoid potential > > integer overflow". > > > > Merging just the input validation patches would be possible too, but I > > prefer > > to merge the corresponding tests so the code is exercised by the CI. > > Is this series still open? I don't see it in master. The Coverity issue is still unfixed, at any rate... -- PMM
Re: [PATCH v2 0/8] hw/sd: sd: Erase operation and other fixes
On 2/16/21 4:02 PM, Bin Meng wrote: > From: Bin Meng > > This includes several fixes related to erase operation of a SD card. > Bin Meng (8): > hw/sd: sd: Fix address check in sd_erase() > hw/sd: sd: Only SDSC cards support CMD28/29/30 > hw/sd: sd: Fix CMD30 response type > hw/sd: sd: Move the sd_block_{read,write} and macros ahead > hw/sd: sd: Skip write protect groups check in sd_erase() for high > capacity cards > hw/sd: sd: Actually perform the erase operation > hw/sd: sd: Skip write protect groups check in CMD24/25 for high > capacity cards > hw/sd: sd: Bypass the RCA check for CMD13 in SPI mode > > hw/sd/sd.c | 99 +++--- > 1 file changed, 64 insertions(+), 35 deletions(-) Thanks, patches 1-5 and 7-8 applied to sdmmc-next tree.
Re: [PATCH] hw/intc/loongson_liointc: Fix per core ISR handling
This patch has been reviewed and fixes a Coverity issue; Philippe, are you planning to take it through your MIPS tree? -- PMM On Tue, 12 Jan 2021 at 01:28, Jiaxun Yang wrote: > > Per core ISR is a set of 32-bit registers spaced by 8 bytes. > This patch fixed calculation of it's size and also added check > of alignment at reading & writing. > > Signed-off-by: Jiaxun Yang > --- > hw/intc/loongson_liointc.c | 16 +--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/hw/intc/loongson_liointc.c b/hw/intc/loongson_liointc.c > index f823d484e0..cc11b544cb 100644 > --- a/hw/intc/loongson_liointc.c > +++ b/hw/intc/loongson_liointc.c > @@ -41,7 +41,7 @@ > #define R_IEN_CLR 0x2c > #define R_ISR_SIZE 0x8 > #define R_START 0x40 > -#define R_END 0x64 > +#define R_END (R_START + R_ISR_SIZE * NUM_CORES) > > struct loongson_liointc { > SysBusDevice parent_obj; > @@ -125,7 +125,12 @@ liointc_read(void *opaque, hwaddr addr, unsigned int > size) > } > > if (addr >= R_START && addr < R_END) { > -int core = (addr - R_START) / R_ISR_SIZE; > +hwaddr offset = addr - R_START; > +int core = offset / R_ISR_SIZE; > + > +if (offset % R_ISR_SIZE) { > +goto out; > +} > r = p->per_core_isr[core]; > goto out; > } > @@ -169,7 +174,12 @@ liointc_write(void *opaque, hwaddr addr, > } > > if (addr >= R_START && addr < R_END) { > -int core = (addr - R_START) / R_ISR_SIZE; > +hwaddr offset = addr - R_START; > +int core = offset / R_ISR_SIZE; > + > +if (offset % R_ISR_SIZE) { > +goto out; > +} > p->per_core_isr[core] = value; > goto out; > } > -- > 2.30.0 > >
[PATCH 2/2] hw/timer/renesas_tmr: Fix use of uninitialized data in read_tcnt()
The read_tcnt() function calculates the TCNT register values for the two channels of the timer module; it sets these up in the local tcnt[] array, and eventually returns either one or both of them, depending on whether the access is 8 or 16 bits. However, not all of the code paths through this function set both elements of this array: if the guest has programmed the TCCR.CSS register fields to values which are either documented as not to be used or which QEMU does not implement, then the function will return uninitialized data. (This was spotted by Coverity.) Add the missing CSS cases to this code, so that we return a consistent value instead of uninitialized data, and so the code structure indicates what's happening. Fixes: CID 1429976 Signed-off-by: Peter Maydell --- hw/timer/renesas_tmr.c | 19 +++ 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/hw/timer/renesas_tmr.c b/hw/timer/renesas_tmr.c index 22260aaaba5..eed39917fec 100644 --- a/hw/timer/renesas_tmr.c +++ b/hw/timer/renesas_tmr.c @@ -46,7 +46,9 @@ REG8(TCCR, 10) FIELD(TCCR, CSS, 3, 2) FIELD(TCCR, TMRIS, 7, 1) +#define CSS_EXTERNAL 0x00 #define CSS_INTERNAL 0x01 +#define CSS_INVALID 0x02 #define CSS_CASCADING 0x03 #define CCLR_A0x01 #define CCLR_B0x02 @@ -130,13 +132,20 @@ static uint16_t read_tcnt(RTMRState *tmr, unsigned size, int ch) if (delta > 0) { tmr->tick = now; -if (FIELD_EX8(tmr->tccr[1], TCCR, CSS) == CSS_INTERNAL) { +switch (FIELD_EX8(tmr->tccr[1], TCCR, CSS)) { +case CSS_INTERNAL: /* timer1 count update */ elapsed = elapsed_time(tmr, 1, delta); if (elapsed >= 0x100) { ovf = elapsed >> 8; } tcnt[1] = tmr->tcnt[1] + (elapsed & 0xff); +break; +case CSS_INVALID: /* guest error to have set this */ +case CSS_EXTERNAL: /* QEMU doesn't implement these */ +case CSS_CASCADING: +tcnt[1] = tmr->tcnt[1]; +break; } switch (FIELD_EX8(tmr->tccr[0], TCCR, CSS)) { case CSS_INTERNAL: @@ -144,9 +153,11 @@ static uint16_t read_tcnt(RTMRState *tmr, unsigned size, int ch) tcnt[0] = tmr->tcnt[0] + elapsed; break; case CSS_CASCADING: -if (ovf > 0) { -tcnt[0] = tmr->tcnt[0] + ovf; -} +tcnt[0] = tmr->tcnt[0] + ovf; +break; +case CSS_INVALID: /* guest error to have set this */ +case CSS_EXTERNAL: /* QEMU doesn't implement this */ +tcnt[0] = tmr->tcnt[0]; break; } } else { -- 2.20.1
[PATCH 0/2] hw/timer/renesas_tmr: Fix use of uninitialized data
This patchseries fixes a use-of-uninitialized-data spotted by Coverity (CID 1429976). Patch 1 just tweaks some constant names for values of the TCCR.CSS register field, since patch 2 needs to add some more defines for the other possible values of the field. Patch 2 is the bugfix proper; the use-uninitialized happens if the guest programs TCCR.CSS to values which are either prohibited in the h/w datasheet, or valid but corresponding to behaviour not currently implemented by QEMU. (Yes, I could have added LOG_UNIMP and/or LOG_GUEST_ERROR when the TCCR is written by the guest; it didn't really seem worth the effort to me.) thanks -- PMM Peter Maydell (2): hw/timer/renesas_tmr: Prefix constants for CSS values with CSS_ hw/timer/renesas_tmr: Fix use of uninitialized data in read_tcnt() hw/timer/renesas_tmr.c | 33 ++--- 1 file changed, 22 insertions(+), 11 deletions(-) -- 2.20.1
[PATCH 1/2] hw/timer/renesas_tmr: Prefix constants for CSS values with CSS_
The #defines INTERNAL and CASCADING represent different possible values for the TCCR.CSS register field; prefix them with CSS_ to make this more obvious, before we add more defines to represent the other possible values of the field in the next commit. Signed-off-by: Peter Maydell --- hw/timer/renesas_tmr.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/timer/renesas_tmr.c b/hw/timer/renesas_tmr.c index e03a8155b2b..22260aaaba5 100644 --- a/hw/timer/renesas_tmr.c +++ b/hw/timer/renesas_tmr.c @@ -46,8 +46,8 @@ REG8(TCCR, 10) FIELD(TCCR, CSS, 3, 2) FIELD(TCCR, TMRIS, 7, 1) -#define INTERNAL 0x01 -#define CASCADING 0x03 +#define CSS_INTERNAL 0x01 +#define CSS_CASCADING 0x03 #define CCLR_A0x01 #define CCLR_B0x02 @@ -72,7 +72,7 @@ static void update_events(RTMRState *tmr, int ch) /* event not happened */ return ; } -if (FIELD_EX8(tmr->tccr[0], TCCR, CSS) == CASCADING) { +if (FIELD_EX8(tmr->tccr[0], TCCR, CSS) == CSS_CASCADING) { /* cascading mode */ if (ch == 1) { tmr->next[ch] = none; @@ -130,7 +130,7 @@ static uint16_t read_tcnt(RTMRState *tmr, unsigned size, int ch) if (delta > 0) { tmr->tick = now; -if (FIELD_EX8(tmr->tccr[1], TCCR, CSS) == INTERNAL) { +if (FIELD_EX8(tmr->tccr[1], TCCR, CSS) == CSS_INTERNAL) { /* timer1 count update */ elapsed = elapsed_time(tmr, 1, delta); if (elapsed >= 0x100) { @@ -139,11 +139,11 @@ static uint16_t read_tcnt(RTMRState *tmr, unsigned size, int ch) tcnt[1] = tmr->tcnt[1] + (elapsed & 0xff); } switch (FIELD_EX8(tmr->tccr[0], TCCR, CSS)) { -case INTERNAL: +case CSS_INTERNAL: elapsed = elapsed_time(tmr, 0, delta); tcnt[0] = tmr->tcnt[0] + elapsed; break; -case CASCADING: +case CSS_CASCADING: if (ovf > 0) { tcnt[0] = tmr->tcnt[0] + ovf; } @@ -330,7 +330,7 @@ static uint16_t issue_event(RTMRState *tmr, int ch, int sz, qemu_irq_pulse(tmr->cmia[ch]); } if (sz == 8 && ch == 0 && -FIELD_EX8(tmr->tccr[1], TCCR, CSS) == CASCADING) { +FIELD_EX8(tmr->tccr[1], TCCR, CSS) == CSS_CASCADING) { tmr->tcnt[1]++; timer_events(tmr, 1); } @@ -362,7 +362,7 @@ static void timer_events(RTMRState *tmr, int ch) uint16_t tcnt; tmr->tcnt[ch] = read_tcnt(tmr, 1, ch); -if (FIELD_EX8(tmr->tccr[0], TCCR, CSS) != CASCADING) { +if (FIELD_EX8(tmr->tccr[0], TCCR, CSS) != CSS_CASCADING) { tmr->tcnt[ch] = issue_event(tmr, ch, 8, tmr->tcnt[ch], tmr->tcora[ch], -- 2.20.1
Re: [PATCH v2 5/8] hw/sd: sd: Skip write protect groups check in sd_erase() for high capacity cards
On 2/16/21 4:02 PM, Bin Meng wrote: > From: Bin Meng > > High capacity cards don't support write protection hence we should > not preform the write protect groups check in sd_erase() for them. > > Signed-off-by: Bin Meng > > --- > > Changes in v2: > - new patch: sd: Skip write protect groups check in sd_erase() for high > capacity card > > hw/sd/sd.c | 18 -- > 1 file changed, 12 insertions(+), 6 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 6/8] hw/sd: sd: Actually perform the erase operation
On 2/16/21 4:02 PM, Bin Meng wrote: > From: Bin Meng > > At present the sd_erase() does not erase the requested range of card > data to 0xFFs. Let's make the erase operation actually happen. > > Signed-off-by: Bin Meng > > --- > > Changes in v2: > - honor the write protection bits for SDSC cards > > hw/sd/sd.c | 22 ++ > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index f1f98bdec3..b386f16fcb 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -766,6 +766,9 @@ static void sd_erase(SDState *sd) > uint64_t erase_start = sd->erase_start; > uint64_t erase_end = sd->erase_end; > bool sdsc = true; > +uint64_t wpnum; > +uint64_t erase_addr; > +int erase_len = 1 << HWBLOCK_SHIFT; > > trace_sdcard_erase(sd->erase_start, sd->erase_end); > if (sd->erase_start == INVALID_ADDRESS > @@ -794,17 +797,20 @@ static void sd_erase(SDState *sd) > sd->erase_end = INVALID_ADDRESS; > sd->csd[14] |= 0x40; > > -/* Only SDSC cards support write protect groups */ > -if (sdsc) { > -erase_start = sd_addr_to_wpnum(erase_start); > -erase_end = sd_addr_to_wpnum(erase_end); > - > -for (i = erase_start; i <= erase_end; i++) { > -assert(i < sd->wpgrps_size); > -if (test_bit(i, sd->wp_groups)) { > +memset(sd->data, 0xff, erase_len); > +erase_addr = erase_start; > +for (i = 0; i <= (erase_end - erase_start) / erase_len; i++) { > +if (sdsc) { > +/* Only SDSC cards support write protect groups */ > +wpnum = sd_addr_to_wpnum(erase_addr); > +assert(wpnum < sd->wpgrps_size); > +if (test_bit(wpnum, sd->wp_groups)) { > sd->card_status |= WP_ERASE_SKIP; > +continue; So if a group is protected, you skip it but don't increase erase_addr. If G#4 is protected and G#5 isn't, when you check G#5 you end erasing G#4. > } > } > +BLK_WRITE_BLOCK(erase_addr, erase_len); > +erase_addr += erase_len; > } > } > >
Re: [PATCH v2 7/8] hw/sd: sd: Skip write protect groups check in CMD24/25 for high capacity cards
On 2/16/21 4:02 PM, Bin Meng wrote: > From: Bin Meng > > High capacity cards don't support write protection hence we should > not preform the write protect groups check in CMD24/25 for them. > > Signed-off-by: Bin Meng > > --- > > Changes in v2: > - new patch: sd: Skip write protect groups check in CMD24/25 for high > capacity cards > > hw/sd/sd.c | 14 +- > 1 file changed, 9 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PULL 00/18] QAPI patches patches for 2021-02-18
On Fri, 19 Feb 2021 at 14:49, Markus Armbruster wrote: > > The following changes since commit 91416a4254015e1e3f602f2b241b9ddb7879c10b: > > Merge remote-tracking branch > 'remotes/stsquad/tags/pull-plugin-updates-180221-1' into staging (2021-02-18 > 13:27:03 +) > > are available in the Git repository at: > > git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2021-02-18 > > for you to fetch changes up to 9b77d946990e7497469bb98171b90b4f3ab186a9: > > qapi/introspect.py: set _gen_tree's default ifcond argument to () > (2021-02-18 19:51:14 +0100) > > > QAPI patches patches for 2021-02-18 > > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0 for any user-visible changes. -- PMM
Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse
On Fri, Feb 19, 2021 at 2:00 AM Daniel P. Berrangé wrote: > On Thu, Feb 18, 2021 at 12:15:36PM -0800, Doug Evans wrote: > > The parsing is moved into new function inet_parse_host_and_port. > > This is done in preparation for using the function in net/slirp.c. > > > > Signed-off-by: Doug Evans > > --- > > > > Changes from v3: > > - this patch is new in v4 > > - provides new utility: inet_parse_host_and_port, updates inet_parse > > to use it > > > > include/qemu/sockets.h | 3 ++ > > util/qemu-sockets.c| 94 +++--- > > 2 files changed, 72 insertions(+), 25 deletions(-) > > > > diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h > > index 7d1f813576..f720378a6b 100644 > > --- a/include/qemu/sockets.h > > +++ b/include/qemu/sockets.h > > @@ -31,6 +31,9 @@ int socket_set_fast_reuse(int fd); > > > > int inet_ai_family_from_address(InetSocketAddress *addr, > > Error **errp); > > +const char* inet_parse_host_and_port(const char* str, int terminator, > > + char **addr, char **port, bool > *is_v6, > > + Error **errp); > > int inet_parse(InetSocketAddress *addr, const char *str, Error **errp); > > int inet_connect(const char *str, Error **errp); > > int inet_connect_saddr(InetSocketAddress *saddr, Error **errp); > > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c > > index 8af0278f15..9fca7d9212 100644 > > --- a/util/qemu-sockets.c > > +++ b/util/qemu-sockets.c > > @@ -615,44 +615,88 @@ static int inet_parse_flag(const char *flagname, > const char *optstr, bool *val, > > return 0; > > } > > > > -int inet_parse(InetSocketAddress *addr, const char *str, Error **errp) > > +/* > > + * Parse an inet host and port as "host:port". > > + * Terminator may be '\0'. > > + * The syntax for ipv4 addresses is: address:port. > > + * The syntax for ipv6 addresses is: [address]:port. > > It also supports > >"The syntax for hostnames is hostname:port > > > + * On success, returns a pointer to the terminator. Space for the > address and > > + * port is malloced and stored in *host, *port, the caller must free. > > + * *is_v6 indicates whether the address is ipv4 or ipv6. If ipv6 then > the > > + * surrounding [] brackets are removed. > > When is_v6 is true, it indicates that a numeric ipv6 address was given. > When false either a numberic ipv4 address or hostname was given. > > > + * On failure NULL is returned with the error stored in *errp. > > + */ > > +const char* inet_parse_host_and_port(const char* str, int terminator, > > + char **hostp, char **portp, bool > *is_v6, > > + Error **errp) > > { > > -const char *optstr, *h; > > +const char *terminator_ptr = strchr(str, terminator); > > +g_autofree char *buf = NULL; > > char host[65]; > > char port[33]; > > -int to; > > -int pos; > > -char *begin; > > > > -memset(addr, 0, sizeof(*addr)); > > +if (terminator_ptr == NULL) { > > +/* If the terminator isn't found then use the entire string. */ > > +terminator_ptr = str + strlen(str); > > +} > > +buf = g_strndup(str, terminator_ptr - str); > > > > -/* parse address */ > > -if (str[0] == ':') { > > -/* no host given */ > > -host[0] = '\0'; > > -if (sscanf(str, ":%32[^,]%n", port, ) != 1) { > > -error_setg(errp, "error parsing port in address '%s'", str); > > -return -1; > > -} > > > > -} else if (str[0] == '[') { > > +if (buf[0] == '[') { > > /* IPv6 addr */ > > -if (sscanf(str, "[%64[^]]]:%32[^,]%n", host, port, ) != 2) { > > -error_setg(errp, "error parsing IPv6 address '%s'", str); > > -return -1; > > +if (buf[1] == ']') { > > +/* sscanf %[ doesn't recognize empty contents. */ > > +host[0] = '\0'; > > +if (sscanf(buf, "[]:%32s", port) != 1) { > > +error_setg(errp, "error parsing IPv6 host:port '%s'", > buf); > > +return NULL; > > +} > > This is introducing new functionality to the parser. Current callers > let empty string ":port" be used for both ipv4 and ipv6, based > on whether the flags ",ipv4[=on|off],ipv6[=on|off]" later follow. > We're creating a new utility subroutine: Let's decide what the API is for it. The fact that inet_parse is passed additional parameters to specify ipv4 vs ipv6 is not something this new subroutine should care about. I presume you want an explicit way to represent an empty ipv6 hostname > to avoid changing semantics for existing slirp CLI args, where the > existing ":port" exclusively means ipv4. IIC, this is also why you > needed to introduce the "is_v6" flag, because any non-empty address > can be reliably parsed without needing this flag. > Actually, no. The "is_v6" flag is needed
Re: [PATCH v2 3/8] hw/sd: sd: Fix CMD30 response type
On 2/16/21 4:02 PM, Bin Meng wrote: > From: Bin Meng > > Per the "Physical Layer Specification Version 8.00", table 4-26 > (SD mode) and table 7-3 (SPI mode) command descriptions, CMD30 > response type is R1, not R1b. > > Fixes: a1bb27b1e98a ("SD card emulation initial implementation") > Signed-off-by: Bin Meng > > --- > > Changes in v2: > - new patch: sd: Fix CMD30 response type > > hw/sd/sd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [RFC PATCH 3/5] tests: add a sdhci reproducer
On 2/18/21 10:12 PM, Alexander Bulekov wrote: > This patch serves as an example of a file generated with the > ./scripts/oss-fuzz/output_reproducer.py script: > The source file in this patch was generated like this: > > $ wget https://paste.debian.net/plain/118513 -O /tmp/trace > $ export QEMU_ARGS="-nographic -machine accel=qtest -m 512M \ > -nodefaults -device sdhci-pci,sd-spec-version=3 -drive \ > if=sd,index=0,file=null-co://,format=raw,id=mydrive \ > -device sd-card,drive=mydrive -qtest stdio" > $ export QEMU_PATH=./qemu-system-i386 > $ ./scripts/oss-fuzz/output_reproducer.py \ > -owner "Alexander Bulekov " /tmp/trace | \ > clang-format -style="{BasedOnStyle: llvm, IndentWidth: 4, \ > ColumnLimit: 90, BreakBeforeBraces: Linux}" > ../tests/qtest/fuzz-sdhci.c > > Signed-off-by: Alexander Bulekov > --- > tests/qtest/fuzz-sdhci.c | 90 > tests/qtest/meson.build | 2 + > 2 files changed, 92 insertions(+) > create mode 100644 tests/qtest/fuzz-sdhci.c ... > diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build > index c83bc211b6..97caf84443 100644 > --- a/tests/qtest/meson.build > +++ b/tests/qtest/meson.build > @@ -56,6 +56,8 @@ qtests_i386 = \ > 'rtc-test', > 'i440fx-test', > 'fuzz-test', > + 'fuzz-sdhci', > + 'sdhci-test', This line ^ belongs to the next patch. > 'fw_cfg-test', > 'device-plug-test', > 'drive_del-test', >
[PULL 5/8] spice-app: avoid crash when core spice module doesn't loaded
From: Bruce Rogers When qemu is built with modules, but a given module doesn't load qemu should handle that gracefully. When ui-spice-core.so isn't able to be loaded and qemu is invoked with -display spice-app or -spice, qemu will dereference a null pointer. With this change we check the pointer before dereferencing and error out in a normal way. Signed-off-by: Bruce Rogers Reviewed-by: Marc-André Lureau Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210213032318.346093-1-brog...@suse.com> Signed-off-by: Gerd Hoffmann --- ui/spice-app.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ui/spice-app.c b/ui/spice-app.c index 026124ef56a0..4325ac2d9c54 100644 --- a/ui/spice-app.c +++ b/ui/spice-app.c @@ -129,6 +129,7 @@ static void spice_app_atexit(void) static void spice_app_display_early_init(DisplayOptions *opts) { QemuOpts *qopts; +QemuOptsList *list; GError *err = NULL; if (opts->has_full_screen) { @@ -159,11 +160,16 @@ static void spice_app_display_early_init(DisplayOptions *opts) exit(1); } } +list = qemu_find_opts("spice"); +if (list == NULL) { +error_report("spice-app missing spice support"); +exit(1); +} type_register(_vc_type_info); sock_path = g_strjoin("", app_dir, "/", "spice.sock", NULL); -qopts = qemu_opts_create(qemu_find_opts("spice"), NULL, 0, _abort); +qopts = qemu_opts_create(list, NULL, 0, _abort); qemu_opt_set(qopts, "disable-ticketing", "on", _abort); qemu_opt_set(qopts, "unix", "on", _abort); qemu_opt_set(qopts, "addr", sock_path, _abort); -- 2.29.2
[PULL 3/8] ui/cocoa: Support unique keys of JIS keyboards
From: Akihiko Odaki Signed-off-by: Akihiko Odaki Message-Id: <20210212000404.28413-1-akihiko.od...@gmail.com> Signed-off-by: Gerd Hoffmann --- ui/cocoa.m | 7 +++ 1 file changed, 7 insertions(+) diff --git a/ui/cocoa.m b/ui/cocoa.m index 13fba8103e1a..78fcfeaf04b7 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -240,6 +240,13 @@ const int mac_to_qkeycode_map[] = { [kVK_F14] = Q_KEY_CODE_SCROLL_LOCK, [kVK_F15] = Q_KEY_CODE_PAUSE, +// JIS keyboards only +[kVK_JIS_Yen] = Q_KEY_CODE_YEN, +[kVK_JIS_Underscore] = Q_KEY_CODE_RO, +[kVK_JIS_KeypadComma] = Q_KEY_CODE_KP_COMMA, +[kVK_JIS_Eisu] = Q_KEY_CODE_MUHENKAN, +[kVK_JIS_Kana] = Q_KEY_CODE_HENKAN, + /* * The eject and volume keys can't be used here because they are handled at * a lower level than what an Application can see. -- 2.29.2
[PULL 8/8] ui/console: Remove dpy_gl_ctx_get_current
From: Akihiko Odaki It is not used, and it is unlikely that a new use case will emerge anytime soon because the scope of OpenGL contexts are limited due to the nature of the frontend, VirGL, processing simple commands from the guest. Remove the function and ease implementing a new OpenGL backend a little. Signed-off-by: Akihiko Odaki Message-Id: <20210219094702.90789-1-akihiko.od...@gmail.com> Signed-off-by: Gerd Hoffmann --- include/ui/gtk.h | 1 - ui/gtk-gl-area.c | 5 - 2 files changed, 6 deletions(-) diff --git a/include/ui/gtk.h b/include/ui/gtk.h index 3c1cd98db8b1..5ae0ad60a600 100644 --- a/include/ui/gtk.h +++ b/include/ui/gtk.h @@ -147,7 +147,6 @@ void gd_gl_area_scanout_disable(DisplayChangeListener *dcl); void gd_gl_area_scanout_flush(DisplayChangeListener *dcl, uint32_t x, uint32_t y, uint32_t w, uint32_t h); void gtk_gl_area_init(void); -QEMUGLContext gd_gl_area_get_current_context(DisplayChangeListener *dcl); int gd_gl_area_make_current(DisplayChangeListener *dcl, QEMUGLContext ctx); diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c index e7ca73c7b1b3..4e8ee88b9b39 100644 --- a/ui/gtk-gl-area.c +++ b/ui/gtk-gl-area.c @@ -239,11 +239,6 @@ void gtk_gl_area_init(void) display_opengl = 1; } -QEMUGLContext gd_gl_area_get_current_context(DisplayChangeListener *dcl) -{ -return gdk_gl_context_get_current(); -} - int gd_gl_area_make_current(DisplayChangeListener *dcl, QEMUGLContext ctx) { -- 2.29.2
[PULL 6/8] ui/cocoa: Interpret left button down as is when command is pressed
From: Akihiko Odaki Old Macs were not equipped with mice with an ability to perform "right clicks" and ui/cocoa interpreted left button down with left command key pressed as right button down as a workaround. The workaround has an obvious downside: you cannot tell the guest that the left button is down while the left command key is pressed. Today, Macs has trackpads, Apple Mice, or Magic Mice. They are capable to emulate right clicks with gestures, which also allows to perform right clicks on "BootCamp" OSes like Windows. By removing the workaround, we overcome its downside, and provide a behavior consistent with BootCamp. Signed-off-by: Akihiko Odaki Message-Id: <20210212000706.28616-1-akihiko.od...@gmail.com> Signed-off-by: Gerd Hoffmann --- ui/cocoa.m | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index eab4bfe7c8ae..13f19bece14d 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -835,11 +835,7 @@ QemuCocoaView *cocoaView; mouse_event = true; break; case NSEventTypeLeftMouseDown: -if ([event modifierFlags] & NSEventModifierFlagCommand) { -buttons |= MOUSE_EVENT_RBUTTON; -} else { -buttons |= MOUSE_EVENT_LBUTTON; -} +buttons |= MOUSE_EVENT_LBUTTON; mouse_event = true; break; case NSEventTypeRightMouseDown: @@ -851,11 +847,7 @@ QemuCocoaView *cocoaView; mouse_event = true; break; case NSEventTypeLeftMouseDragged: -if ([event modifierFlags] & NSEventModifierFlagCommand) { -buttons |= MOUSE_EVENT_RBUTTON; -} else { -buttons |= MOUSE_EVENT_LBUTTON; -} +buttons |= MOUSE_EVENT_LBUTTON; mouse_event = true; break; case NSEventTypeRightMouseDragged: -- 2.29.2
[PULL 7/8] ui/cocoa: Statically allocate dcl
From: Akihiko Odaki There is no need of dynamic allocation as dcl is a small singleton. Static allocation reduces code size and makes hacking with ui/cocoa a bit easier. Signed-off-by: Akihiko Odaki Message-Id: <20210219084419.90181-1-akihiko.od...@gmail.com> Signed-off-by: Gerd Hoffmann --- ui/cocoa.m | 65 ++ 1 file changed, 31 insertions(+), 34 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index 13f19bece14d..0ef5fdf3b7a3 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -72,8 +72,24 @@ typedef struct { int height; } QEMUScreen; +static void cocoa_update(DisplayChangeListener *dcl, + int x, int y, int w, int h); + +static void cocoa_switch(DisplayChangeListener *dcl, + DisplaySurface *surface); + +static void cocoa_refresh(DisplayChangeListener *dcl); + NSWindow *normalWindow, *about_window; -static DisplayChangeListener *dcl; +static const DisplayChangeListenerOps dcl_ops = { +.dpy_name = "cocoa", +.dpy_gfx_update = cocoa_update, +.dpy_gfx_switch = cocoa_switch, +.dpy_refresh = cocoa_refresh, +}; +static DisplayChangeListener dcl = { +.ops = _ops, +}; static int last_buttons; static int cursor_hide = 1; @@ -607,15 +623,15 @@ QemuCocoaView *cocoaView; // Toggle the stored state. modifiers_state[keycode] = !modifiers_state[keycode]; // Send a keyup or keydown depending on the state. -qemu_input_event_send_key_qcode(dcl->con, keycode, modifiers_state[keycode]); +qemu_input_event_send_key_qcode(dcl.con, keycode, modifiers_state[keycode]); } - (void) toggleStatefulModifier: (int)keycode { // Toggle the stored state. modifiers_state[keycode] = !modifiers_state[keycode]; // Generate keydown and keyup. -qemu_input_event_send_key_qcode(dcl->con, keycode, true); -qemu_input_event_send_key_qcode(dcl->con, keycode, false); +qemu_input_event_send_key_qcode(dcl.con, keycode, true); +qemu_input_event_send_key_qcode(dcl.con, keycode, false); } // Does the work of sending input to the monitor @@ -799,7 +815,7 @@ QemuCocoaView *cocoaView; } if (qemu_console_is_graphic(NULL)) { -qemu_input_event_send_key_qcode(dcl->con, keycode, true); +qemu_input_event_send_key_qcode(dcl.con, keycode, true); } else { [self handleMonitorInput: event]; } @@ -814,7 +830,7 @@ QemuCocoaView *cocoaView; } if (qemu_console_is_graphic(NULL)) { -qemu_input_event_send_key_qcode(dcl->con, keycode, false); +qemu_input_event_send_key_qcode(dcl.con, keycode, false); } break; case NSEventTypeMouseMoved: @@ -892,9 +908,9 @@ QemuCocoaView *cocoaView; /* Determine if this is a scroll up or scroll down event */ buttons = ([event deltaY] > 0) ? INPUT_BUTTON_WHEEL_UP : INPUT_BUTTON_WHEEL_DOWN; -qemu_input_queue_btn(dcl->con, buttons, true); +qemu_input_queue_btn(dcl.con, buttons, true); qemu_input_event_sync(); -qemu_input_queue_btn(dcl->con, buttons, false); +qemu_input_queue_btn(dcl.con, buttons, false); qemu_input_event_sync(); } /* @@ -922,7 +938,7 @@ QemuCocoaView *cocoaView; [INPUT_BUTTON_MIDDLE] = MOUSE_EVENT_MBUTTON, [INPUT_BUTTON_RIGHT] = MOUSE_EVENT_RBUTTON }; -qemu_input_update_buttons(dcl->con, bmap, last_buttons, buttons); +qemu_input_update_buttons(dcl.con, bmap, last_buttons, buttons); last_buttons = buttons; } if (isMouseGrabbed) { @@ -932,12 +948,12 @@ QemuCocoaView *cocoaView; * clicks in the titlebar. */ if ([self screenContainsPoint:p]) { -qemu_input_queue_abs(dcl->con, INPUT_AXIS_X, p.x, 0, screen.width); -qemu_input_queue_abs(dcl->con, INPUT_AXIS_Y, screen.height - p.y, 0, screen.height); +qemu_input_queue_abs(dcl.con, INPUT_AXIS_X, p.x, 0, screen.width); +qemu_input_queue_abs(dcl.con, INPUT_AXIS_Y, screen.height - p.y, 0, screen.height); } } else { -qemu_input_queue_rel(dcl->con, INPUT_AXIS_X, (int)[event deltaX]); -qemu_input_queue_rel(dcl->con, INPUT_AXIS_Y, (int)[event deltaY]); +qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, (int)[event deltaX]); +qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, (int)[event deltaY]); } } else { return false; @@ -1006,7 +1022,7 @@ QemuCocoaView *cocoaView; for (index = 0; index < max_index; index++) { if (modifiers_state[index]) {
[PATCH v5 3/4] Jobs based on custom runners: docs and gitlab-runner setup playbook
To have the jobs dispatched to custom runners, gitlab-runner must be installed, active as a service and properly configured. The variables file and playbook introduced here should help with those steps. The playbook introduced here covers a number of different Linux distributions and FreeBSD, and are intended to provide a reproducible environment. Signed-off-by: Cleber Rosa Reviewed-by: Daniel P. Berrangé --- docs/devel/ci.rst | 58 ++ scripts/ci/setup/.gitignore| 1 + scripts/ci/setup/gitlab-runner.yml | 65 ++ scripts/ci/setup/vars.yml.template | 13 ++ 4 files changed, 137 insertions(+) create mode 100644 scripts/ci/setup/.gitignore create mode 100644 scripts/ci/setup/gitlab-runner.yml create mode 100644 scripts/ci/setup/vars.yml.template diff --git a/docs/devel/ci.rst b/docs/devel/ci.rst index a556558435..9f9c4bd3f9 100644 --- a/docs/devel/ci.rst +++ b/docs/devel/ci.rst @@ -56,3 +56,61 @@ To run the playbook, execute:: cd scripts/ci/setup ansible-playbook -i inventory build-environment.yml + +gitlab-runner setup and registration + + +The gitlab-runner agent needs to be installed on each machine that +will run jobs. The association between a machine and a GitLab project +happens with a registration token. To find the registration token for +your repository/project, navigate on GitLab's web UI to: + + * Settings (the gears like icon), then + * CI/CD, then + * Runners, and click on the "Expand" button, then + * Under "Set up a specific Runner manually", look for the value under + "Use the following registration token during setup" + +Copy the ``scripts/ci/setup/vars.yml.template`` file to +``scripts/ci/setup/vars.yml``. Then, set the +``gitlab_runner_registration_token`` variable to the value obtained +earlier. + +.. note:: gitlab-runner is not available from the standard location + for all OS and architectures combinations. For some systems, + a custom build may be necessary. Some builds are avaiable + at https://cleber.fedorapeople.org/gitlab-runner/ and this + URI may be used as a value on ``vars.yml`` + +To run the playbook, execute:: + + cd scripts/ci/setup + ansible-playbook -i inventory gitlab-runner.yml + +Following the registration, it's necessary to configure the runner tags, +and optionally other configurations on the GitLab UI. Navigate to: + + * Settings (the gears like icon), then + * CI/CD, then + * Runners, and click on the "Expand" button, then + * "Runners activated for this project", then + * Click on the "Edit" icon (next to the "Lock" Icon) + +Under tags, add values matching the jobs a runner should run. For a +Ubuntu 20.04 aarch64 system, the tags should be set as:: + + ubuntu_20.04,aarch64 + +Because the job definition at ``.gitlab-ci.d/custom-runners.yml`` +would contain:: + + ubuntu-20.04-aarch64-all: + tags: + - ubuntu_20.04 + - aarch64 + +It's also recommended to: + + * increase the "Maximum job timeout" to something like ``2h`` + * uncheck the "Run untagged jobs" check box + * give it a better Description diff --git a/scripts/ci/setup/.gitignore b/scripts/ci/setup/.gitignore new file mode 100644 index 00..f112d05dd0 --- /dev/null +++ b/scripts/ci/setup/.gitignore @@ -0,0 +1 @@ +vars.yml \ No newline at end of file diff --git a/scripts/ci/setup/gitlab-runner.yml b/scripts/ci/setup/gitlab-runner.yml new file mode 100644 index 00..ab1944965f --- /dev/null +++ b/scripts/ci/setup/gitlab-runner.yml @@ -0,0 +1,65 @@ +--- +- name: Installation of gitlab-runner + hosts: all + vars_files: +- vars.yml + tasks: +- debug: +msg: 'Checking for a valid GitLab registration token' + failed_when: "gitlab_runner_registration_token == 'PLEASE_PROVIDE_A_VALID_TOKEN'" + +- name: Checks the availability of official gitlab-runner builds in the archive + uri: +url: https://s3.amazonaws.com/gitlab-runner-downloads/v{{ gitlab_runner_version }}/binaries/gitlab-runner-linux-386 +method: HEAD +status_code: + - 200 + - 403 + register: gitlab_runner_available_archive + +- name: Update base url + set_fact: +gitlab_runner_base_url: https://s3.amazonaws.com/gitlab-runner-downloads/v{{ gitlab_runner_version }}/binaries/gitlab-runner- + when: gitlab_runner_available_archive.status == 200 +- debug: +msg: Base gitlab-runner url is {{ gitlab_runner_base_url }} + +- name: Create a group for the gitlab-runner service + group: +name: gitlab-runner + +- name: Create a user for the gitlab-runner service + user: +user: gitlab-runner +group: gitlab-runner +comment: GitLab Runner +home: /home/gitlab-runner +shell: /bin/bash + +- name: Remove the .bash_logout file when on Ubuntu systems + file: +path:
[PULL 2/8] spice: flush drawing before notifying client
From: Marc-André Lureau This solves the client having slow/outdated VGA/2D console. It's a regression introduced when the code was switched to render it via opengl in commit 4423184376d ("spice/gl: render DisplaySurface via opengl") Signed-off-by: Marc-André Lureau Message-Id: <20210216092056.2301293-2-marcandre.lur...@redhat.com> Signed-off-by: Gerd Hoffmann --- ui/spice-display.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/spice-display.c b/ui/spice-display.c index d562c6408405..ad93b953a90c 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -846,6 +846,7 @@ static void spice_gl_refresh(DisplayChangeListener *dcl) graphic_hw_update(dcl->con); if (ssd->gl_updates && ssd->have_surface) { qemu_spice_gl_block(ssd, true); +glFlush(); cookie = (uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_GL_DRAW_DONE, 0); spice_qxl_gl_draw_async(>qxl, 0, 0, surface_width(ssd->ds), -- 2.29.2
[PULL 4/8] ui/cocoa: Do not copy members of pixman image
From: Akihiko Odaki The old CocoaView had an idea of synchronizing the host window configuration and the guest screen configuration. Here, the guest screen actually means pixman image given ui/cocoa display implementation. However, [CocoaView -drawRect:] directly interacts with the pixman image buffer in reality. There is no such distinction of "host" and "guest." This change removes the "host" configuration and let drawRect consistently have the direct reference to pixman image. It allows to get rid of the error-prone "sync" and reduce code size a bit. Signed-off-by: Akihiko Odaki Message-Id: <20210212000629.28551-1-akihiko.od...@gmail.com> Signed-off-by: Gerd Hoffmann --- ui/cocoa.m | 42 -- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index 78fcfeaf04b7..eab4bfe7c8ae 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -70,8 +70,6 @@ typedef struct { int width; int height; -int bitsPerComponent; -int bitsPerPixel; } QEMUScreen; NSWindow *normalWindow, *about_window; @@ -291,7 +289,6 @@ static void handleAnyDeviceErrors(Error * err) QEMUScreen screen; NSWindow *fullScreenWindow; float cx,cy,cw,ch,cdx,cdy; -CGDataProviderRef dataProviderRef; pixman_image_t *pixman_image; BOOL modifiers_state[256]; BOOL isMouseGrabbed; @@ -338,8 +335,6 @@ QemuCocoaView *cocoaView; self = [super initWithFrame:frameRect]; if (self) { -screen.bitsPerComponent = 8; -screen.bitsPerPixel = 32; screen.width = frameRect.size.width; screen.height = frameRect.size.height; @@ -351,8 +346,7 @@ QemuCocoaView *cocoaView; { COCOA_DEBUG("QemuCocoaView: dealloc\n"); -if (dataProviderRef) { -CGDataProviderRelease(dataProviderRef); +if (pixman_image) { pixman_image_unref(pixman_image); } @@ -431,18 +425,28 @@ QemuCocoaView *cocoaView; CGContextSetShouldAntialias (viewContextRef, NO); // draw screen bitmap directly to Core Graphics context -if (!dataProviderRef) { +if (!pixman_image) { // Draw request before any guest device has set up a framebuffer: // just draw an opaque black rectangle CGContextSetRGBFillColor(viewContextRef, 0, 0, 0, 1.0); CGContextFillRect(viewContextRef, NSRectToCGRect(rect)); } else { +int w = pixman_image_get_width(pixman_image); +int h = pixman_image_get_height(pixman_image); +int bitsPerPixel = PIXMAN_FORMAT_BPP(pixman_image_get_format(pixman_image)); +int bitsPerComponent = DIV_ROUND_UP(bitsPerPixel, 8) * 2; +CGDataProviderRef dataProviderRef = CGDataProviderCreateWithData( +NULL, +pixman_image_get_data(pixman_image), +w * 4 * h, +NULL +); CGImageRef imageRef = CGImageCreate( -screen.width, //width -screen.height, //height -screen.bitsPerComponent, //bitsPerComponent -screen.bitsPerPixel, //bitsPerPixel -(screen.width * (screen.bitsPerComponent/2)), //bytesPerRow +w, //width +h, //height +bitsPerComponent, //bitsPerComponent +bitsPerPixel, //bitsPerPixel +(w * (bitsPerComponent/2)), //bytesPerRow #ifdef __LITTLE_ENDIAN__ CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), //colorspace for OS X >= 10.4 kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst, @@ -465,7 +469,7 @@ QemuCocoaView *cocoaView; [self getRectsBeingDrawn: count:]; for (i = 0; i < rectCount; i++) { clipRect.origin.x = rectList[i].origin.x / cdx; -clipRect.origin.y = (float)screen.height - (rectList[i].origin.y + rectList[i].size.height) / cdy; +clipRect.origin.y = (float)h - (rectList[i].origin.y + rectList[i].size.height) / cdy; clipRect.size.width = rectList[i].size.width / cdx; clipRect.size.height = rectList[i].size.height / cdy; clipImageRef = CGImageCreateWithImageInRect( @@ -476,6 +480,7 @@ QemuCocoaView *cocoaView; CGImageRelease (clipImageRef); } CGImageRelease (imageRef); +CGDataProviderRelease(dataProviderRef); } } @@ -518,7 +523,6 @@ QemuCocoaView *cocoaView; int w = pixman_image_get_width(image); int h = pixman_image_get_height(image); -pixman_format_code_t image_format = pixman_image_get_format(image); /* cdx == 0 means this is our very first surface, in which case we need * to recalculate the content dimensions even if it happens to be the size * of the initial empty window. @@ -536,17 +540,11 @@ QemuCocoaView *cocoaView; } // update screenBuffer -if (dataProviderRef) { -CGDataProviderRelease(dataProviderRef); +if (pixman_image) { pixman_image_unref(pixman_image); } -//sync
[PULL 1/8] spice: flush on GL update before notifying client
From: Marc-André Lureau Since the introduction of spice/virgl support in commit 474114b7 ("spice: add opengl/virgl/dmabuf support"), the drawing isn't being flushed before notifying the client. This results in outdated/sluggish drawing on client side, in particular when using the Linux console. Signed-off-by: Marc-André Lureau Message-Id: <20210216092056.2301293-1-marcandre.lur...@redhat.com> Signed-off-by: Gerd Hoffmann --- ui/spice-display.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/spice-display.c b/ui/spice-display.c index 6f32b66a6e75..d562c6408405 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -1087,6 +1087,7 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl, trace_qemu_spice_gl_update(ssd->qxl.id, w, h, x, y); qemu_spice_gl_block(ssd, true); +glFlush(); cookie = (uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_GL_DRAW_DONE, 0); spice_qxl_gl_draw_async(>qxl, x, y, w, h, cookie); } -- 2.29.2
[PATCH v5 4/4] Jobs based on custom runners: add job definitions for QEMU's machines
The QEMU project has two machines (aarch64 and s390x) that can be used for jobs that do build and run tests. This introduces those jobs, which are a mapping of custom scripts used for the same purpose. Signed-off-by: Cleber Rosa Reviewed-by: Daniel P. Berrangé --- .gitlab-ci.d/custom-runners.yml | 204 1 file changed, 204 insertions(+) diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml index 3004da2bda..a9166c82a2 100644 --- a/.gitlab-ci.d/custom-runners.yml +++ b/.gitlab-ci.d/custom-runners.yml @@ -12,3 +12,207 @@ # strategy. variables: GIT_SUBMODULE_STRATEGY: recursive + +# All ubuntu-18.04 jobs should run successfully in an environment +# setup by the scripts/ci/setup/build-environment.yml task +# "Install basic packages to build QEMU on Ubuntu 18.04/20.04" +ubuntu-18.04-s390x-all-linux-static: + allow_failure: true + needs: [] + stage: build + tags: + - ubuntu_18.04 + - s390x + rules: + - if: '$CI_COMMIT_BRANCH =~ /^staging/' + script: + # --disable-libssh is needed because of https://bugs.launchpad.net/qemu/+bug/1838763 + # --disable-glusterfs is needed because there's no static version of those libs in distro supplied packages + - mkdir build + - cd build + - ../configure --enable-debug --static --disable-system --disable-glusterfs --disable-libssh + - make --output-sync -j`nproc` + - make --output-sync -j`nproc` check V=1 + - make --output-sync -j`nproc` check-tcg V=1 + +ubuntu-18.04-s390x-all: + allow_failure: true + needs: [] + stage: build + tags: + - ubuntu_18.04 + - s390x + rules: + - if: '$CI_COMMIT_BRANCH =~ /^staging/' + script: + - mkdir build + - cd build + - ../configure --disable-libssh + - make --output-sync -j`nproc` + - make --output-sync -j`nproc` check V=1 + +ubuntu-18.04-s390x-alldbg: + allow_failure: true + needs: [] + stage: build + tags: + - ubuntu_18.04 + - s390x + rules: + - if: '$CI_COMMIT_BRANCH =~ /^staging/' + script: + - mkdir build + - cd build + - ../configure --enable-debug --disable-libssh + - make clean + - make --output-sync -j`nproc` + - make --output-sync -j`nproc` check V=1 + +ubuntu-18.04-s390x-clang: + allow_failure: true + needs: [] + stage: build + tags: + - ubuntu_18.04 + - s390x + rules: + - if: '$CI_COMMIT_BRANCH =~ /^staging/' + script: + - mkdir build + - cd build + - ../configure --disable-libssh --cc=clang --cxx=clang++ --enable-sanitizers + - make --output-sync -j`nproc` + - make --output-sync -j`nproc` check V=1 + +ubuntu-18.04-s390x-tci: + allow_failure: true + needs: [] + stage: build + tags: + - ubuntu_18.04 + - s390x + rules: + - if: '$CI_COMMIT_BRANCH =~ /^staging/' + script: + - mkdir build + - cd build + - ../configure --disable-libssh --enable-tcg-interpreter + - make --output-sync -j`nproc` + +ubuntu-18.04-s390x-notcg: + allow_failure: true + needs: [] + stage: build + tags: + - ubuntu_18.04 + - s390x + rules: + - if: '$CI_COMMIT_BRANCH =~ /^staging/' + script: + - mkdir build + - cd build + - ../configure --disable-libssh --disable-tcg + - make --output-sync -j`nproc` + - make --output-sync -j`nproc` check V=1 + +# All ubuntu-20.04 jobs should run successfully in an environment +# setup by the scripts/ci/setup/qemu/build-environment.yml task +# "Install basic packages to build QEMU on Ubuntu 18.04/20.04" +ubuntu-20.04-aarch64-all-linux-static: + allow_failure: true + needs: [] + stage: build + tags: + - ubuntu_20.04 + - aarch64 + rules: + - if: '$CI_COMMIT_BRANCH =~ /^staging/' + script: + # --disable-libssh is needed because of https://bugs.launchpad.net/qemu/+bug/1838763 + # --disable-glusterfs is needed because there's no static version of those libs in distro supplied packages + - mkdir build + - cd build + - ../configure --enable-debug --static --disable-system --disable-glusterfs --disable-libssh + - make --output-sync -j`nproc` + - make --output-sync -j`nproc` check V=1 + - make --output-sync -j`nproc` check-tcg V=1 + +ubuntu-20.04-aarch64-all: + allow_failure: true + needs: [] + stage: build + tags: + - ubuntu_20.04 + - aarch64 + rules: + - if: '$CI_COMMIT_BRANCH =~ /^staging/' + script: + - mkdir build + - cd build + - ../configure --disable-libssh + - make --output-sync -j`nproc` + - make --output-sync -j`nproc` check V=1 + +ubuntu-20.04-aarch64-alldbg: + allow_failure: true + needs: [] + stage: build + tags: + - ubuntu_20.04 + - aarch64 + rules: + - if: '$CI_COMMIT_BRANCH =~ /^staging/' + script: + - mkdir build + - cd build + - ../configure --enable-debug --disable-libssh + - make clean + - make --output-sync -j`nproc` + - make --output-sync -j`nproc` check V=1 + +ubuntu-20.04-aarch64-clang: + allow_failure: true + needs: [] + stage: build + tags: + - ubuntu_20.04 + - aarch64 + rules: + - if: '$CI_COMMIT_BRANCH =~ /^staging/' + script: + - mkdir build + - cd build + - ../configure --disable-libssh --cc=clang --cxx=clang++ --enable-sanitizers + - make --output-sync -j`nproc` + - make --output-sync -j`nproc` check V=1 + +ubuntu-20.04-aarch64-tci: +
[PULL 0/8] Ui 20210219 patches
The following changes since commit c79f01c9450bcf90c08a77f13fbf67bdba59a316: Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-hex-20210218' in= to staging (2021-02-18 16:33:36 +) are available in the Git repository at: git://git.kraxel.org/qemu tags/ui-20210219-pull-request for you to fetch changes up to 075e7a5b7f3c640823fce76c8dab503c42f0d7f6: ui/console: Remove dpy_gl_ctx_get_current (2021-02-19 15:07:14 +0100) ui: spice bugfixes. ui: first batch of cocoa updates. Akihiko Odaki (5): ui/cocoa: Support unique keys of JIS keyboards ui/cocoa: Do not copy members of pixman image ui/cocoa: Interpret left button down as is when command is pressed ui/cocoa: Statically allocate dcl ui/console: Remove dpy_gl_ctx_get_current Bruce Rogers (1): spice-app: avoid crash when core spice module doesn't loaded Marc-Andr=C3=A9 Lureau (2): spice: flush on GL update before notifying client spice: flush drawing before notifying client include/ui/gtk.h | 1 - ui/gtk-gl-area.c | 5 -- ui/spice-app.c | 8 ++- ui/spice-display.c | 2 + ui/cocoa.m | 126 + 5 files changed, 69 insertions(+), 73 deletions(-) --=20 2.29.2
[PATCH v5 0/4] GitLab Custom Runners and Jobs (was: QEMU Gating CI)
TL;DR: this should allow the QEMU maintainer to push to the staging branch, and have custom jobs running on the project's aarch64 and s390x machines. Jobs in this version are allowed to fail, to allow for the inclusion of the novel machines/jobs without CI disruption. Simple usage looks like: git push remote staging ./scripts/ci/gitlab-pipeline-status --verbose --wait Long version: The idea about a public facing Gating CI for QEMU was summarized in an RFC[1]. Since then, it was decided that a simpler version should be attempted first. At this point, there are two specific runners (an aarch64 and an s390x) registered with GitLab, at https://gitlab.com/qemu-project, currently setup to the "qemu" repository. Changes from v4: - Fixed typo in docs/devel/ci.rst, s/maintanance/maintenance/ (Thomas) - Removed "[local]" group from inventory file (Erik) - Removed sections from the playbooks which *would* be applied on hardware/OS that are currently not available to QEMU - Removed duplicated "here" on documentation (Thomas) - Moved description of current jobs, and possible direction of future jobs to the patch description (Thomas) - Remove comments around "when" conditions (Andrea) - Switch to always use explicit lists on "when" blocks (Andrea) - Switch from using module "apt" to using generic action module "package", which involved adding a new task to update the apt cache (Andrea) - Fix playbook indentation in the non-s390x package installation task (Andrea) - Changed gitlab-runner tags examples from FreeBSD to Ubuntu, which is covered by jobs added on this version - Fixed typo in commit message s/s390/s390x/ (Phil) - Allow all custom-runner jobs to fail at this time - Cleared "Reviewed-by" in one patch due to large changes Changes requested in v4 but *not* seen here due to sections of the playbook being removed: - Replace SDL-devel for SDL2-devel on CentOS, according to 5ed7ca3 (Thomas) - Correct missing step 10 on the FreeBSD gitlab-runner installation instructions (Erik) Changes from v3: - Applied changes to match <20201014135416.1290679-1-pbonz...@redhat.com>, that is, added ninja-build to "build-environment.yml" list of packages and enabled PowerTools repository on CentOS 8. Changes from v2: - The overall idea of "Gating CI" has been re-worded "custom runners", given that the other jobs running on shared runners are also considered gating (Daniel) - Fixed wording and typos on the documentation, including: * update -> up to date (Erik) * a different set of CI jobs -> different CI jobs (Erik) * Pull requests will only be merged -> code will only be merged (Stefan) * Setup -> set up (Stefan) * them -> they (Stefan) * the -> where the (Stefan) * dropped "in the near future" (Stefan) - Changed comment on "build-environment.yml" regarding the origin of the package list (Stefan) - Removed inclusion of "vars.yml" from "build-environment.yml", given that no external variable is used there - Updated package list in "build-environment.yml" from current dockerfiles - Tested "build-environment" on Fedora 31 and 32 (in addition to Fedora 30), and noted that it's possible to use it on those distros - Moved CI documentation from "testing.rst" to its own file (Phillipe) - Split "GitLab Gating CI: initial set of jobs, documentation and scripts" into (Phillipe): 1) Basic documentation and configuration (gitlab-ci.yml) placeholder 2) Playbooks for setting up a build environment 3) Playbooks for setting up gitlab-runner 4) Actual GitLab CI jobs configuration - Set custom jobs to be on the "build" stage, given that they combine build and test. - Set custom jobs to not depend on any other job, so they can start right away. - Set rules for starting jobs so that all pushing to any branch that start with name "staging". This allows the project maintainer to use the "push to staging" workflow, while also allowing others to generate similar jobs. If this project has configured custom runners, the jobs will run, if not, the pipeline will be marked as "stuck". - Changed "scripts" on custom jobs to follow the now common pattern (on other jobs) of creating a "build" directory. Changes from v1: - Added jobs that require specific GitLab runners already available (Ubuntu 20.04 on aarch64, and Ubuntu 18.04 on s390x) - Removed jobs that require specific GitLab runners not yet available (Fedora 30, FreeBSD 12.1) - Updated documentation - Added copyright and license to new scripts - Moved script to from "contrib" to "scripts/ci/" - Moved setup playbooks form "contrib" to "scripts/ci/setup" - Moved "gating.yml" to ".gitlab-ci.d" directory - Removed "staging" only branch restriction on jobs defined in ".gitlab-ci.yml", assumes that the additional jobs on the staging branch running on the freely available gitlab shared runner are positive - Dropped patches 1-3 (already merged) - Simplified amount of version specifity on
[PATCH v5 2/4] Jobs based on custom runners: build environment docs and playbook
To run basic jobs on custom runners, the environment needs to be properly set up. The most common requirement is having the right packages installed. The playbook introduced here covers the QEMU's project s390x and aarch64 machines. At the time this is being proposed, those machines have already had this playbook applied to them. Signed-off-by: Cleber Rosa --- docs/devel/ci.rst | 30 ++ scripts/ci/setup/build-environment.yml | 76 ++ scripts/ci/setup/inventory | 1 + 3 files changed, 107 insertions(+) create mode 100644 scripts/ci/setup/build-environment.yml create mode 100644 scripts/ci/setup/inventory diff --git a/docs/devel/ci.rst b/docs/devel/ci.rst index 585b7bf4b8..a556558435 100644 --- a/docs/devel/ci.rst +++ b/docs/devel/ci.rst @@ -26,3 +26,33 @@ gitlab-runner, is called a "custom runner". The GitLab CI jobs definition for the custom runners are located under:: .gitlab-ci.d/custom-runners.yml + +Machine Setup Howto +--- + +For all Linux based systems, the setup can be mostly automated by the +execution of two Ansible playbooks. Start by adding your machines to +the ``inventory`` file under ``scripts/ci/setup``, such as this:: + + fully.qualified.domain + other.machine.hostname + +You may need to set some variables in the inventory file itself. One +very common need is to tell Ansible to use a Python 3 interpreter on +those hosts. This would look like:: + + fully.qualified.domain ansible_python_interpreter=/usr/bin/python3 + other.machine.hostname ansible_python_interpreter=/usr/bin/python3 + +Build environment +~ + +The ``scripts/ci/setup/build-environment.yml`` Ansible playbook will +set up machines with the environment needed to perform builds and run +QEMU tests. It covers a number of different Linux distributions and +FreeBSD. + +To run the playbook, execute:: + + cd scripts/ci/setup + ansible-playbook -i inventory build-environment.yml diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml new file mode 100644 index 00..0197e0a48b --- /dev/null +++ b/scripts/ci/setup/build-environment.yml @@ -0,0 +1,76 @@ +--- +- name: Installation of basic packages to build QEMU + hosts: all + tasks: +- name: Update apt cache + apt: +update_cache: yes + when: +- ansible_facts['distribution'] == 'Ubuntu' + +- name: Install basic packages to build QEMU on Ubuntu 18.04/20.04 + package: +name: +# Originally from tests/docker/dockerfiles/ubuntu1804.docker + - ccache + - clang + - gcc + - gettext + - git + - glusterfs-common + - libaio-dev + - libattr1-dev + - libbrlapi-dev + - libbz2-dev + - libcacard-dev + - libcap-ng-dev + - libcurl4-gnutls-dev + - libdrm-dev + - libepoxy-dev + - libfdt-dev + - libgbm-dev + - libgtk-3-dev + - libibverbs-dev + - libiscsi-dev + - libjemalloc-dev + - libjpeg-turbo8-dev + - liblzo2-dev + - libncurses5-dev + - libncursesw5-dev + - libnfs-dev + - libnss3-dev + - libnuma-dev + - libpixman-1-dev + - librados-dev + - librbd-dev + - librdmacm-dev + - libsasl2-dev + - libsdl2-dev + - libseccomp-dev + - libsnappy-dev + - libspice-protocol-dev + - libssh-dev + - libusb-1.0-0-dev + - libusbredirhost-dev + - libvdeplug-dev + - libvte-2.91-dev + - libzstd-dev + - make + - ninja-build + - python3-yaml + - python3-sphinx + - sparse + - xfslibs-dev +state: present + when: +- ansible_facts['distribution'] == 'Ubuntu' + +- name: Install packages to build QEMU on Ubuntu 18.04/20.04 on non-s390x + package: +name: + - libspice-server-dev + - libxen-dev +state: present + when: +- ansible_facts['distribution'] == 'Ubuntu' +- ansible_facts['architecture'] != 's390x' diff --git a/scripts/ci/setup/inventory b/scripts/ci/setup/inventory new file mode 100644 index 00..2fbb50c4a8 --- /dev/null +++ b/scripts/ci/setup/inventory @@ -0,0 +1 @@ +localhost -- 2.25.4
[PATCH v5 1/4] Jobs based on custom runners: documentation and configuration placeholder
As described in the included documentation, the "custom runner" jobs extend the GitLab CI jobs already in place. One of their primary goals of catching and preventing regressions on a wider number of host systems than the ones provided by GitLab's shared runners. This sets the stage in which other community members can add their own machine configuration documentation/scripts, and accompanying job definitions. As a general rule, those newly added contributed jobs should run as "non-gating", until their reliability is verified (AKA "allow_failure: true"). Signed-off-by: Cleber Rosa --- .gitlab-ci.d/custom-runners.yml | 14 ++ .gitlab-ci.yml | 1 + docs/devel/ci.rst | 28 docs/devel/index.rst| 1 + 4 files changed, 44 insertions(+) create mode 100644 .gitlab-ci.d/custom-runners.yml create mode 100644 docs/devel/ci.rst diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml new file mode 100644 index 00..3004da2bda --- /dev/null +++ b/.gitlab-ci.d/custom-runners.yml @@ -0,0 +1,14 @@ +# The CI jobs defined here require GitLab runners installed and +# registered on machines that match their operating system names, +# versions and architectures. This is in contrast to the other CI +# jobs that are intended to run on GitLab's "shared" runners. + +# Different than the default approach on "shared" runners, based on +# containers, the custom runners have no such *requirement*, as those +# jobs should be capable of running on operating systems with no +# compatible container implementation, or no support from +# gitlab-runner. To avoid problems that gitlab-runner can cause while +# reusing the GIT repository, let's enable the recursive submodule +# strategy. +variables: + GIT_SUBMODULE_STRATEGY: recursive diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 8b6d495288..ae19442e93 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -12,6 +12,7 @@ include: - local: '/.gitlab-ci.d/opensbi.yml' - local: '/.gitlab-ci.d/containers.yml' - local: '/.gitlab-ci.d/crossbuilds.yml' + - local: '/.gitlab-ci.d/custom-runners.yml' .native_build_job_template: _build_job_definition stage: build diff --git a/docs/devel/ci.rst b/docs/devel/ci.rst new file mode 100644 index 00..585b7bf4b8 --- /dev/null +++ b/docs/devel/ci.rst @@ -0,0 +1,28 @@ +== +CI +== + +QEMU has configurations enabled for a number of different CI services. +The most up to date information about them and their status can be +found at:: + + https://wiki.qemu.org/Testing/CI + +Jobs on Custom Runners +== + +Besides the jobs run under the various CI systems listed before, there +are a number additional jobs that will run before an actual merge. +These use the same GitLab CI's service/framework already used for all +other GitLab based CI jobs, but rely on additional systems, not the +ones provided by GitLab as "shared runners". + +The architecture of GitLab's CI service allows different machines to +be set up with GitLab's "agent", called gitlab-runner, which will take +care of running jobs created by events such as a push to a branch. +Here, the combination of a machine, properly configured with GitLab's +gitlab-runner, is called a "custom runner". + +The GitLab CI jobs definition for the custom runners are located under:: + + .gitlab-ci.d/custom-runners.yml diff --git a/docs/devel/index.rst b/docs/devel/index.rst index 22854e334d..b178448a91 100644 --- a/docs/devel/index.rst +++ b/docs/devel/index.rst @@ -23,6 +23,7 @@ Contents: migration atomics stable-process + ci qtest decodetree secure-coding-practices -- 2.25.4
Re: [PATCH v4 1/4] slirp: Advance libslirp submodule to add ipv6 host-forward support
On Fri, Feb 19, 2021 at 1:38 AM Daniel P. Berrangé wrote: > On Thu, Feb 18, 2021 at 12:15:35PM -0800, Doug Evans wrote: > > FWIW, normally when QEMU updates libslirp, the commit message is > set to contain the "git shortlog old..new" output > Ah. In this case I'm not sure what to do as QEMU master is using Libslirp stable-4.2 branch (at least in QEMU's libslirp.git). Samuel, please let me know what should happen here. I may need some hand holding to come up with The Right patch to submit. I think you know what patches are needed here, but I don't know what I should be submitting in this 1/4 patch of the series. > > > Signed-off-by: Doug Evans > > --- > > > > Changes from v3: > > - pick up latest libslirp patch to reject ipv6 addr-any for guest address > > - libslirp currently only provides a stateless DHCPv6 server, which > means > > it can't know in advance what the guest's IP address is, and thus > > cannot do the "addr-any -> guest ip address" translation that is done > > for ipv4 > > > > Changes from v2: > > - this patch is new in v3, split out from v2 > > > > slirp | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/slirp b/slirp > > index 8f43a99191..26ae658a83 16 > > --- a/slirp > > +++ b/slirp > > @@ -1 +1 @@ > > -Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece > > +Subproject commit 26ae658a83eeca16780cf5615c8247cbb151c3fa > > -- > > 2.30.0.617.g56c4b15f3c-goog > > > > Regards, > Daniel > -- > |: https://berrange.com -o- > https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- > https://fstop138.berrange.com :| > |: https://entangle-photo.org-o- > https://www.instagram.com/dberrange :| > >
[PATCH v2] bsd-user: Add new maintainers
From: Warner Losh The FreeBSD project has a number of enhancements to bsd-user. Add myself as maintainer and Kyle Evans as a reviewer. Also add our github repo. Signed-off-by: Warner Losh Reviewed-by: Thomas Huth --- MAINTAINERS | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 66354e6e49..141e01075b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2884,9 +2884,12 @@ F: thunk.c F: accel/tcg/user-exec*.c BSD user -S: Orphan +M: Warner Losh +R: Kyle Evans +S: Maintained F: bsd-user/ -F: default-configs/*-bsd-user.mak +F: default-configs/targets/*-bsd-user.mak +T: git https://github.com/qemu-bsd-user/qemu-bsd-user bsd-user-rebase-3.1 Linux user M: Laurent Vivier -- 2.30.0
[PATCH] FreeBSD: Upgrade to 12.2 release
From: Warner Losh FreeBSD 12.1 has reached end of life. Use 12.2 instead so that FreeBSD's project's packages will work. Signed-off-by: Warner Losh --- tests/vm/freebsd | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/vm/freebsd b/tests/vm/freebsd index 09f3ee6cb8..c5886f6500 100755 --- a/tests/vm/freebsd +++ b/tests/vm/freebsd @@ -24,8 +24,8 @@ class FreeBSDVM(basevm.BaseVM): name = "freebsd" arch = "x86_64" -link = "https://download.freebsd.org/ftp/releases/ISO-IMAGES/12.1/FreeBSD-12.1-RELEASE-amd64-disc1.iso.xz; -csum = "7394c3f60a1e236e7bd3a05809cf43ae39a3b8e5d42d782004cf2f26b1cfcd88" +link = "https://download.freebsd.org/ftp/releases/ISO-IMAGES/12.2/FreeBSD-12.2-RELEASE-amd64-disc1.iso.xz; +csum = "a4530246cafbf1dd42a9bd3ea441ca9a78a6a0cd070278cbdf63f3a6f803ecae" size = "20G" pkgs = [ # build tools -- 2.30.0
Re: [PATCH v3 7/7] spapr_drc.c: use DRC reconfiguration to cleanup DIMM unplug state
On 2/16/21 11:31 PM, David Gibson wrote: On Thu, Feb 11, 2021 at 07:52:46PM -0300, Daniel Henrique Barboza wrote: Handling errors in memory hotunplug in the pSeries machine is more complex than any other device type, because there are all the complications that other devices has, and more. [...] diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index ecce8abf14..4bcded4a1a 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3575,6 +3575,36 @@ static SpaprDimmState *spapr_recover_pending_dimm_state(SpaprMachineState *ms, return spapr_pending_dimm_unplugs_add(ms, avail_lmbs, dimm); } +void spapr_clear_pending_dimm_unplug_state(SpaprMachineState *spapr, + PCDIMMDevice *dimm) +{ +SpaprDimmState *ds = spapr_pending_dimm_unplugs_find(spapr, dimm); +SpaprDrc *drc; +uint32_t nr_lmbs; +uint64_t size, addr_start, addr; +int i; + +if (ds) { +spapr_pending_dimm_unplugs_remove(spapr, ds); +} Hrm... how would !ds arise? Could this just be an assert? !ds would appear if we do not assert g_assert(drc->dev) down there, where you suggested down below that a malicious/buggy code would trigger it, for example. With that assert in place then this less likely to occcur. I guess what I can do here is: - remove the g_assert(drc->dev) from down below, since it's more related to the logic of this function; - here, check if drc->dev is NULL. Return doing nothing if that's the case (all the function relies on drc->dev being valid); - if drc->dev is not NULL, then we can g_assert(ds) and proceed with the rest of the function This way we become a little more tolerant on drc->dev being NULL, but if drc->dev is valid we will expect a unplug dimm state to always exist and assert it. Thanks, DHB + +size = memory_device_get_region_size(MEMORY_DEVICE(dimm), _abort); +nr_lmbs = size / SPAPR_MEMORY_BLOCK_SIZE; + +addr_start = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP, + _abort); + +addr = addr_start; +for (i = 0; i < nr_lmbs; i++) { +drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB, + addr / SPAPR_MEMORY_BLOCK_SIZE); +g_assert(drc); + +drc->unplug_requested = false; +addr += SPAPR_MEMORY_BLOCK_SIZE; +} +} + /* Callback to be called during DRC release. */ void spapr_lmb_release(DeviceState *dev) { diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index c143bfb6d3..eae941233a 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -1230,6 +1230,20 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); +/* + * This indicates that the kernel is reconfiguring a LMB due to + * a failed hotunplug. Clear the pending unplug state for the whole + * DIMM. + */ +if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_LMB && +drc->unplug_requested) { + +/* This really shouldn't happen in this point, but ... */ +g_assert(drc->dev); I'm a little worried that a buggy or malicious guest could trigger this assert. + +spapr_clear_pending_dimm_unplug_state(spapr, PC_DIMM(drc->dev)); +} + if (!drc->fdt) { void *fdt; int fdt_size; diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index ccbeeca1de..5bcc8f3bb8 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -847,6 +847,8 @@ int spapr_hpt_shift_for_ramsize(uint64_t ramsize); int spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, Error **errp); void spapr_clear_pending_events(SpaprMachineState *spapr); void spapr_clear_pending_hotplug_events(SpaprMachineState *spapr); +void spapr_clear_pending_dimm_unplug_state(SpaprMachineState *spapr, + PCDIMMDevice *dimm); int spapr_max_server_number(SpaprMachineState *spapr); void spapr_store_hpte(PowerPCCPU *cpu, hwaddr ptex, uint64_t pte0, uint64_t pte1);
Re: [PATCH v13 0/5] UFFD write-tracking migration/snapshots
> Am 19.02.2021 um 22:14 schrieb David Hildenbrand : > > >>> Am 19.02.2021 um 22:10 schrieb Peter Xu : >>> >>> On Fri, Feb 19, 2021 at 03:50:52PM -0500, Peter Xu wrote: >>> Andrey, >>> On Fri, Feb 19, 2021 at 09:57:37AM +0300, Andrey Gruzdev wrote: For the discards that happen before snapshot is started, I need to dig into Linux and QEMU virtio-baloon code more to get clear with it. >>> >>> Yes it's very tricky on how the error could trigger. >>> >>> Let's think of below sequence: >>> >>> - Start a guest with init_on_free=1 set and also a virtio-balloon device >>> >>> - Guest frees a page P and zeroed it (since init_on_free=1). Now P contains >>> all zeros. >>> >>> - Virtio-balloon reports this page to host, MADV_DONTNEED sent, then this >>> page is dropped on the host. >>> >>> - Start live snapshot, wr-protect all pages (but not including page P >>> because >>> it's currently missing). Let's call it $SNAPSHOT1. >>> >>> - Guest does alloc_page(__GFP_ZERO), accidentally fetching this page P and >>> returned >>> >>> - So far, page P is still all zero (which is good!), then guest uses page P >>> and writes data to it (say, now P has data P1 rather than all zeros). >>> >>> - Live snapshot saves page P, which content P1 rather than all zeros. >>> >>> - Live snapshot completed. Saved as $SNAPSHOT1. >>> >>> Then when load snapshot $SNAPSHOT1, we'll have P contains data P1. After >>> snapshot loaded, when guest allocate again with alloc_page(__GFP_ZERO) on >>> this >>> page P, since guest kernel "thought" this page is all-zero already so >>> memzero() >>> is skipped even if __GFP_ZERO is provided. Then this page P (with content >>> P1) >>> got returned for the alloc_page(__GFP_ZERO) even if __GFP_ZERO set. That >>> could >>> break the caller of alloc_page(). >>> Anyhow I'm quite sure that adding global MISSING handler for snapshotting is too heavy and not really needed. >>> >>> UFFDIO_ZEROCOPY installs a zero pfn and that should be all of it. There'll >>> definitely be overhead, but it may not be that huge as imagined. Live >>> snapshot >>> is great in that we have point-in-time image of guest without stopping the >>> guest, so taking slightly longer time won't be a huge loss to us too. >>> >>> Actually we can also think of other ways to work around it. One way is we >>> can >>> pre-fault all guest pages before wr-protect. Note that we don't need to >>> write >>> to the guest page because read would suffice, since uffd-wp would also work >>> with zero pfn. It's just that this workaround won't help on saving snapshot >>> disk space, but it seems working. It would be great if you have other >>> workarounds, maybe as you said UFFDIO_ZEROCOPY is not the only route. >> >> Wait.. it actually seems to also solve the disk usage issue.. :) >> >> We should just need to make sure to prohibit balloon before staring to >> pre-fault read on all guest ram. Seems awkward, but also seems working.. >> Hmm.. > > A shiver just went down my spine. Please don‘t just for the sake of creating > a snapshot. > > (Just imagine you don‘t have a shared zeropage...) ... and I just remembered we read all memory either way. Gah. I have some patches to make snapshots fly with virtio-mem so exactly that won‘t happen. But they depend on vfio support, so it might take a while.
Re: [PATCH v13 0/5] UFFD write-tracking migration/snapshots
> Am 19.02.2021 um 22:10 schrieb Peter Xu : > > On Fri, Feb 19, 2021 at 03:50:52PM -0500, Peter Xu wrote: >> Andrey, >> >>> On Fri, Feb 19, 2021 at 09:57:37AM +0300, Andrey Gruzdev wrote: >>> For the discards that happen before snapshot is started, I need to dig into >>> Linux and QEMU virtio-baloon >>> code more to get clear with it. >> >> Yes it's very tricky on how the error could trigger. >> >> Let's think of below sequence: >> >> - Start a guest with init_on_free=1 set and also a virtio-balloon device >> >> - Guest frees a page P and zeroed it (since init_on_free=1). Now P contains >>all zeros. >> >> - Virtio-balloon reports this page to host, MADV_DONTNEED sent, then this >>page is dropped on the host. >> >> - Start live snapshot, wr-protect all pages (but not including page P >> because >>it's currently missing). Let's call it $SNAPSHOT1. >> >> - Guest does alloc_page(__GFP_ZERO), accidentally fetching this page P and >>returned >> >> - So far, page P is still all zero (which is good!), then guest uses page P >>and writes data to it (say, now P has data P1 rather than all zeros). >> >> - Live snapshot saves page P, which content P1 rather than all zeros. >> >> - Live snapshot completed. Saved as $SNAPSHOT1. >> >> Then when load snapshot $SNAPSHOT1, we'll have P contains data P1. After >> snapshot loaded, when guest allocate again with alloc_page(__GFP_ZERO) on >> this >> page P, since guest kernel "thought" this page is all-zero already so >> memzero() >> is skipped even if __GFP_ZERO is provided. Then this page P (with content >> P1) >> got returned for the alloc_page(__GFP_ZERO) even if __GFP_ZERO set. That >> could >> break the caller of alloc_page(). >> >>> Anyhow I'm quite sure that adding global MISSING handler for snapshotting >>> is too heavy and not really needed. >> >> UFFDIO_ZEROCOPY installs a zero pfn and that should be all of it. There'll >> definitely be overhead, but it may not be that huge as imagined. Live >> snapshot >> is great in that we have point-in-time image of guest without stopping the >> guest, so taking slightly longer time won't be a huge loss to us too. >> >> Actually we can also think of other ways to work around it. One way is we >> can >> pre-fault all guest pages before wr-protect. Note that we don't need to >> write >> to the guest page because read would suffice, since uffd-wp would also work >> with zero pfn. It's just that this workaround won't help on saving snapshot >> disk space, but it seems working. It would be great if you have other >> workarounds, maybe as you said UFFDIO_ZEROCOPY is not the only route. > > Wait.. it actually seems to also solve the disk usage issue.. :) > > We should just need to make sure to prohibit balloon before staring to > pre-fault read on all guest ram. Seems awkward, but also seems working.. > Hmm.. A shiver just went down my spine. Please don‘t just for the sake of creating a snapshot. (Just imagine you don‘t have a shared zeropage...) > -- > Peter Xu >
Re: [PATCH v13 0/5] UFFD write-tracking migration/snapshots
On Fri, Feb 19, 2021 at 03:50:52PM -0500, Peter Xu wrote: > Andrey, > > On Fri, Feb 19, 2021 at 09:57:37AM +0300, Andrey Gruzdev wrote: > > For the discards that happen before snapshot is started, I need to dig into > > Linux and QEMU virtio-baloon > > code more to get clear with it. > > Yes it's very tricky on how the error could trigger. > > Let's think of below sequence: > > - Start a guest with init_on_free=1 set and also a virtio-balloon device > > - Guest frees a page P and zeroed it (since init_on_free=1). Now P contains > all zeros. > > - Virtio-balloon reports this page to host, MADV_DONTNEED sent, then this > page is dropped on the host. > > - Start live snapshot, wr-protect all pages (but not including page P > because > it's currently missing). Let's call it $SNAPSHOT1. > > - Guest does alloc_page(__GFP_ZERO), accidentally fetching this page P and > returned > > - So far, page P is still all zero (which is good!), then guest uses page P > and writes data to it (say, now P has data P1 rather than all zeros). > > - Live snapshot saves page P, which content P1 rather than all zeros. > > - Live snapshot completed. Saved as $SNAPSHOT1. > > Then when load snapshot $SNAPSHOT1, we'll have P contains data P1. After > snapshot loaded, when guest allocate again with alloc_page(__GFP_ZERO) on this > page P, since guest kernel "thought" this page is all-zero already so > memzero() > is skipped even if __GFP_ZERO is provided. Then this page P (with content P1) > got returned for the alloc_page(__GFP_ZERO) even if __GFP_ZERO set. That > could > break the caller of alloc_page(). > > > Anyhow I'm quite sure that adding global MISSING handler for snapshotting > > is too heavy and not really needed. > > UFFDIO_ZEROCOPY installs a zero pfn and that should be all of it. There'll > definitely be overhead, but it may not be that huge as imagined. Live > snapshot > is great in that we have point-in-time image of guest without stopping the > guest, so taking slightly longer time won't be a huge loss to us too. > > Actually we can also think of other ways to work around it. One way is we can > pre-fault all guest pages before wr-protect. Note that we don't need to write > to the guest page because read would suffice, since uffd-wp would also work > with zero pfn. It's just that this workaround won't help on saving snapshot > disk space, but it seems working. It would be great if you have other > workarounds, maybe as you said UFFDIO_ZEROCOPY is not the only route. Wait.. it actually seems to also solve the disk usage issue.. :) We should just need to make sure to prohibit balloon before staring to pre-fault read on all guest ram. Seems awkward, but also seems working.. Hmm.. -- Peter Xu
Re: [PATCH v13 0/5] UFFD write-tracking migration/snapshots
Andrey, On Fri, Feb 19, 2021 at 09:57:37AM +0300, Andrey Gruzdev wrote: > For the discards that happen before snapshot is started, I need to dig into > Linux and QEMU virtio-baloon > code more to get clear with it. Yes it's very tricky on how the error could trigger. Let's think of below sequence: - Start a guest with init_on_free=1 set and also a virtio-balloon device - Guest frees a page P and zeroed it (since init_on_free=1). Now P contains all zeros. - Virtio-balloon reports this page to host, MADV_DONTNEED sent, then this page is dropped on the host. - Start live snapshot, wr-protect all pages (but not including page P because it's currently missing). Let's call it $SNAPSHOT1. - Guest does alloc_page(__GFP_ZERO), accidentally fetching this page P and returned - So far, page P is still all zero (which is good!), then guest uses page P and writes data to it (say, now P has data P1 rather than all zeros). - Live snapshot saves page P, which content P1 rather than all zeros. - Live snapshot completed. Saved as $SNAPSHOT1. Then when load snapshot $SNAPSHOT1, we'll have P contains data P1. After snapshot loaded, when guest allocate again with alloc_page(__GFP_ZERO) on this page P, since guest kernel "thought" this page is all-zero already so memzero() is skipped even if __GFP_ZERO is provided. Then this page P (with content P1) got returned for the alloc_page(__GFP_ZERO) even if __GFP_ZERO set. That could break the caller of alloc_page(). > Anyhow I'm quite sure that adding global MISSING handler for snapshotting > is too heavy and not really needed. UFFDIO_ZEROCOPY installs a zero pfn and that should be all of it. There'll definitely be overhead, but it may not be that huge as imagined. Live snapshot is great in that we have point-in-time image of guest without stopping the guest, so taking slightly longer time won't be a huge loss to us too. Actually we can also think of other ways to work around it. One way is we can pre-fault all guest pages before wr-protect. Note that we don't need to write to the guest page because read would suffice, since uffd-wp would also work with zero pfn. It's just that this workaround won't help on saving snapshot disk space, but it seems working. It would be great if you have other workarounds, maybe as you said UFFDIO_ZEROCOPY is not the only route. Thanks, -- Peter Xu
Re: [PATCH] target/arm: Use TCF0 and TFSRE0 for unprivileged tag checks
Patchew URL: https://patchew.org/QEMU/20210219201820.2672077-1-...@google.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210219201820.2672077-1-...@google.com Subject: [PATCH] target/arm: Use TCF0 and TFSRE0 for unprivileged tag checks === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/20210219201820.2672077-1-...@google.com -> patchew/20210219201820.2672077-1-...@google.com Switched to a new branch 'test' 8b335c2 target/arm: Use TCF0 and TFSRE0 for unprivileged tag checks === OUTPUT BEGIN === ERROR: Author email address is mangled by the mailing list #2: Author: Peter Collingbourne via total: 1 errors, 0 warnings, 34 lines checked Commit 8b335c251c00 (target/arm: Use TCF0 and TFSRE0 for unprivileged tag checks) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20210219201820.2672077-1-...@google.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH] target/i386/sev: Ensure sev_fw_errlist is sync with update-linux-headers
On 2/19/21 12:01 PM, Philippe Mathieu-Daudé wrote: Ensure sev_fw_errlist[] is updated after running the update-linux-headers.sh script. Signed-off-by: Philippe Mathieu-Daudé --- Based-on: <20210218151633.215374-1-cku...@redhat.com> --- target/i386/sev.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) Reviewed-by: Connor Kuehl Thanks! Connor
[PATCH] target/arm: Use TCF0 and TFSRE0 for unprivileged tag checks
Section D6.7 of the ARM ARM states: For the purpose of determining Tag Check Fault handling, unprivileged load and store instructions are treated as if executed at EL0 when executed at either: - EL1, when the Effective value of PSTATE.UAO is 0. - EL2, when both the Effective value of HCR_EL2.{E2H, TGE} is {1, 1} and the Effective value of PSTATE.UAO is 0. ARM has confirmed a defect in the pseudocode function AArch64.TagCheckFault that makes it inconsistent with the above wording. The remedy is to adjust references to PSTATE.EL in that function to instead refer to AArch64.AccessUsesEL(acctype), so that unprivileged instructions use SCTLR_EL1.TCF0 and TFSRE0_EL1. The exception type for synchronous tag check faults remains unchanged. This patch implements the described change by partially reverting commits 50244cc76abc and cc97b0019bb5. Signed-off-by: Peter Collingbourne --- target/arm/helper.c | 2 +- target/arm/mte_helper.c | 13 + 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index 0e1a3b9421..b0223bda4c 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -13133,7 +13133,7 @@ static uint32_t rebuild_hflags_a64(CPUARMState *env, int el, int fp_el, if (FIELD_EX32(flags, TBFLAG_A64, UNPRIV) && tbid && !(env->pstate & PSTATE_TCO) -&& (sctlr & SCTLR_TCF) +&& (sctlr & SCTLR_TCF0) && allocation_tag_access_enabled(env, 0, sctlr)) { flags = FIELD_DP32(flags, TBFLAG_A64, MTE0_ACTIVE, 1); } diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c index 1c569336ea..0bbb9ec346 100644 --- a/target/arm/mte_helper.c +++ b/target/arm/mte_helper.c @@ -550,10 +550,14 @@ static void mte_check_fail(CPUARMState *env, uint32_t desc, reg_el = regime_el(env, arm_mmu_idx); sctlr = env->cp15.sctlr_el[reg_el]; -el = arm_current_el(env); -if (el == 0) { +switch (arm_mmu_idx) { +case ARMMMUIdx_E10_0: +case ARMMMUIdx_E20_0: +el = 0; tcf = extract64(sctlr, 38, 2); -} else { +break; +default: +el = reg_el; tcf = extract64(sctlr, 40, 2); } @@ -570,7 +574,8 @@ static void mte_check_fail(CPUARMState *env, uint32_t desc, env->exception.vaddress = dirty_ptr; is_write = FIELD_EX32(desc, MTEDESC, WRITE); -syn = syn_data_abort_no_iss(el != 0, 0, 0, 0, 0, is_write, 0x11); +syn = syn_data_abort_no_iss(arm_current_el(env) != 0, 0, 0, 0, 0, +is_write, 0x11); raise_exception(env, EXCP_DATA_ABORT, syn, exception_target_el(env)); /* noreturn, but fall through to the assert anyway */ -- 2.30.0.617.g56c4b15f3c-goog
Re: [PATCH v3 7/7] spapr_drc.c: use DRC reconfiguration to cleanup DIMM unplug state
On 2/16/21 11:31 PM, David Gibson wrote: On Thu, Feb 11, 2021 at 07:52:46PM -0300, Daniel Henrique Barboza wrote: Handling errors in memory hotunplug in the pSeries machine is more complex than any other device type, because there are all the complications that other devices has, and more. For instance, determining a timeout for a DIMM hotunplug must consider if it's a Hash-MMU or a Radix-MMU guest, because Hash guests takes longer to hotunplug DIMMs. The size of the DIMM is also a factor, given that longer DIMMs naturally takes longer to be hotunplugged from the kernel. And there's also the guest memory usage to be considered: if there's a process that is consuming memory that would be lost by the DIMM unplug, the kernel will postpone the unplug process until the process finishes, and then initiate the regular hotunplug process. The first two considerations are manageable, but the last one is a deal breaker. There is no sane way for the pSeries machine to determine the memory load in the guest when attempting a DIMM hotunplug - and even if there was a way, the guest can start using all the RAM in the middle of the unplug process and invalidate our previous assumptions - and in result we can't even begin to calculate a timeout for the operation. This means that we can't implement a viable timeout mechanism for memory unplug in pSeries. Going back to why we would consider an unplug timeout, the reason is that we can't know if the kernel is giving up the unplug. Turns out that, sometimes, we can. Consider a failed memory hotunplug attempt where the kernel will error out with the following message: 'pseries-hotplug-mem: Memory indexed-count-remove failed, adding any removed LMBs' This happens when there is a LMB that the kernel gave up in removing, and the LMBs marked for removal of the same DIMM are now being added back. This process happens We need to be a little careful about terminology here. From the guest's point of view, there's no such thing as a DIMM, only LMBs. What the guest is doing here is essentially rejecting a single "index + number" DRC unplug request, which corresponds to one DIMM on the qemu side. I'll reword this paragraph to avoid using "DIMM" in the context of the guest kernel. in the pseries kernel in [1], dlpar_memory_remove_by_ic() into dlpar_add_lmb(), and after that update_lmb_associativity_index(). In this function, the kernel is configuring the LMB DRC connector again. Note that this is a valid usage in LOPAR, as stated in section "ibm,configure-connector RTAS Call": 'A subsequent sequence of calls to ibm,configure-connector with the same entry from the “ibm,drc-indexes” or “ibm,drc-info” property will restart the configuration of devices which were not completely configured.' We can use this kernel behavior in our favor. If a DRC connector reconfiguration for a LMB that we marked as unplug pending happens, this indicates that the kernel changed its mind about the unplug and is reasserting that it will keep using the DIMM. In this case, it's safe to assume that the whole DIMM unplug was cancelled. This patch hops into rtas_ibm_configure_connector() and, in the scenario described above, clear the unplug state for the DIMM device. This will not solve all the problems we still have with memory unplug, but it will cover this case where the kernel reconfigures LMBs after a failed unplug. We are a bit more resilient, without using an unreliable timeout, and we didn't make the remaining error cases any worse. I wonder if we could use this as a beginning of a hotplug failure reporting mechanism. As noted, this is explicitly allowed by PAPR and I think in general it makes sense that a configure-connector would re-assert that the guest is using the resource and we can't unplug it. I think it's worth looking into it. The kernel already does that in case of hotunplug failure of LMBs (at least in this particular case), so it's a matter of evaluating how hard it is to do the same for e.g. CPUs. Could we extend guests to do an indicative configure-connector on any unplug it knows it can't complete? Or if configure-connector is too disruptive could we use an (extra) H_SET_INDICATOR to "UNISOLATE" state? If I'm reading right, that should be both permitted and a no-op for existing PAPR implementations, so it should be a pretty safe way to add that indication. A quick look in LOPAR shows that set_indicator can be used to report hotplug failures (which is a surprise to me, I wasn't aware of it): - (Table 13.7, R1-13.5.3.4-4.) For all DR options: If this is a DR operation that involves the user insert- ing a DR entity, then if the firmware can determine that the inserted entity would cause a system disturbance, then the set-indicator RTAS call must not unisolate the entity and must return an error status which is unique to the particular error. - The wording 'would cause a system disturbance' seems generic on purpose, giving the
Re: Can not set high msize with virtio-9p (Was: Re: virtiofs vs 9p performance)
On Fri, Feb 19, 2021 at 06:33:46PM +0100, Christian Schoenebeck wrote: > On Freitag, 19. Februar 2021 17:08:48 CET Vivek Goyal wrote: > > On Fri, Sep 25, 2020 at 10:06:41AM +0200, Christian Schoenebeck wrote: > > > On Freitag, 25. September 2020 00:10:23 CEST Vivek Goyal wrote: > > > > In my testing, with cache=none, virtiofs performed better than 9p in > > > > all the fio jobs I was running. For the case of cache=auto for virtiofs > > > > (with xattr enabled), 9p performed better in certain write workloads. I > > > > have identified root cause of that problem and working on > > > > HANDLE_KILLPRIV_V2 patches to improve WRITE performance of virtiofs > > > > with cache=auto and xattr enabled. > > > > > > Please note, when it comes to performance aspects, you should set a > > > reasonable high value for 'msize' on 9p client side: > > > https://wiki.qemu.org/Documentation/9psetup#msize > > > > Hi Christian, > > > > I am not able to set msize to a higher value. If I try to specify msize > > 16MB, and then read back msize from /proc/mounts, it sees to cap it > > at 512000. Is that intended? > > 9p server side in QEMU does not perform any msize capping. The code in this > case is very simple, it's just what you see in function v9fs_version(): > > https://github.com/qemu/qemu/blob/6de76c5f324904c93e69f9a1e8e4fd0bd6f6b57a/hw/9pfs/9p.c#L1332 > > > $ mount -t 9p -o trans=virtio,version=9p2000.L,cache=none,msize=16777216 > > hostShared /mnt/virtio-9p > > > > $ cat /proc/mounts | grep 9p > > hostShared /mnt/virtio-9p 9p > > rw,sync,dirsync,relatime,access=client,msize=512000,trans=virtio 0 0 > > > > I am using 5.11 kernel. > > Must be something on client (guest kernel) side. I don't see this here with > guest kernel 4.9.0 happening with my setup in a quick test: > > $ cat /etc/mtab | grep 9p > svnRoot / 9p > rw,dirsync,relatime,trans=virtio,version=9p2000.L,msize=104857600,cache=mmap > 0 0 > $ > > Looks like the root cause of your issue is this: > > struct p9_client *p9_client_create(const char *dev_name, char *options) > { > ... > if (clnt->msize > clnt->trans_mod->maxsize) > clnt->msize = clnt->trans_mod->maxsize; > > https://github.com/torvalds/linux/blob/f40ddce88593482919761f74910f42f4b84c004b/net/9p/client.c#L1045 That was introduced by a patch 2011. commit c9ffb05ca5b5098d6ea468c909dd384d90da7d54 Author: Venkateswararao Jujjuri (JV) Date: Wed Jun 29 18:06:33 2011 -0700 net/9p: Fix the msize calculation. msize represents the maximum PDU size that includes P9_IOHDRSZ. You kernel 4.9 is newer than this. So most likely you have this commit too. I will spend some time later trying to debug this. Vivek
Re: [PATCH 0/2] Allwinner H3 fixes for EMAC and acceptance tests
On Fri, Feb 19, 2021 at 07:24:01PM +0100, Philippe Mathieu-Daudé wrote: > > I hope you understand the concern I have is not with you in particular, > and I used your case to start a discussion with the QEMU community. > > FWIW I missed the URL change because I still have the image cached in > Avocado so my testing ran fine. Which makes me wonder... > > Cleber, Willian, should Avocado display information about cached > artifacts? Such "Using artifact downloaded 7 months ago". > As of Avocado 85.0 (currently used in QEMU), it's possible to set the "expire" parameter to "fetch_asset", see: https://avocado-framework.readthedocs.io/en/85.0/api/test/avocado.html#avocado.Test.fetch_asset In this case, if we want assets to not be used if they're are 30 days or older, that could be set to 86400. The expired asset not being used, and then not being able to be fetched again, would cause a test to be canceled. Cache browsing/listing/manipulation using the "avocado assets" command is planned for Avocado 86.0, see: https://github.com/avocado-framework/avocado/issues/4311 > > So what I can do > > instead is: > > > > - update the patch to use github to store the artifacts, and their > > licenses (other tests also use github) > > Until there is better solutions, this is the option I prefer. > +1. Regards, - Cleber. signature.asc Description: PGP signature
Re: [PATCH v2 1/1] css: SCHIB measurement block origin must be aligned
On 2/19/21 2:41 PM, Thomas Huth wrote: On 19/02/2021 14.39, Pierre Morel wrote: The Measurement Block Origin inside the SCHIB is used when Measurement Block format 1 is in used and must be aligned on 64 bytes otherwise an operand exception is recognized when issuing the Modify Sub CHannel (MSCH) instruction. Signed-off-by: Pierre Morel --- target/s390x/ioinst.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c index a412926d27..1ee11522e1 100644 --- a/target/s390x/ioinst.c +++ b/target/s390x/ioinst.c @@ -121,6 +121,12 @@ static int ioinst_schib_valid(SCHIB *schib) if (be32_to_cpu(schib->pmcw.chars) & PMCW_CHARS_MASK_XMWME) { return 0; } + /* for MB format 1 bits 26-31 of word 11 must be 0 */ + /* MBA uses words 10 and 11, it means align on 2**6 */ + if ((be16_to_cpu(schib->pmcw.chars) & PMCW_CHARS_MASK_MBFC) && + (be64_to_cpu(schib->mba) & 0x03fUL)) { + return 0; + } return 1; } Reviewed-by: Thomas Huth Thanks, Pierre -- Pierre Morel IBM Lab Boeblingen
[PATCH 2/4] ui: introduce "password-secret" option for SPICE server
Currently when using SPICE the "password" option provides the password in plain text on the command line. This is insecure as it is visible to all processes on the host. As an alternative, the password can be provided separately via the monitor. This introduces a "password-secret" option which lets the password be provided up front. $QEMU --object secret,id=vncsec0,file=passwd.txt \ --spice port=5901,password-secret=vncsec0 Signed-off-by: Daniel P. Berrangé --- qemu-options.hx | 8 ++-- ui/spice-core.c | 28 ++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index 893d0f500b..ff4ef3b708 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1898,7 +1898,7 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice, " [,tls-ciphers=]\n" " [,tls-channel=[main|display|cursor|inputs|record|playback]]\n" " [,plaintext-channel=[main|display|cursor|inputs|record|playback]]\n" -" [,sasl][,password=][,disable-ticketing]\n" +" [,sasl][,password=][,password-secret=][,disable-ticketing]\n" " [,image-compression=[auto_glz|auto_lz|quic|glz|lz|off]]\n" " [,jpeg-wan-compression=[auto|never|always]]\n" " [,zlib-glz-wan-compression=[auto|never|always]]\n" @@ -1923,9 +1923,13 @@ SRST ``ipv4``; \ ``ipv6``; \ ``unix`` Force using the specified IP version. -``password=`` +``password=`` Set the password you need to authenticate. +``password-secret=`` +Set the ID of the ``secret`` object containing the password +you need to authenticate. + ``sasl`` Require that the client use SASL to authenticate with the spice. The exact choice of authentication method used is controlled diff --git a/ui/spice-core.c b/ui/spice-core.c index beee932f55..353848b244 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -34,6 +34,7 @@ #include "qapi/qapi-events-ui.h" #include "qemu/notify.h" #include "qemu/option.h" +#include "crypto/secret_common.h" #include "migration/misc.h" #include "hw/pci/pci_bus.h" #include "ui/spice-display.h" @@ -415,6 +416,9 @@ static QemuOptsList qemu_spice_opts = { },{ .name = "password", .type = QEMU_OPT_STRING, +},{ +.name = "password-secret", +.type = QEMU_OPT_STRING, },{ .name = "disable-ticketing", .type = QEMU_OPT_BOOL, @@ -636,7 +640,9 @@ void qemu_spice_display_init_done(void) static void qemu_spice_init(void) { QemuOpts *opts = QTAILQ_FIRST(_spice_opts.head); -const char *password, *str, *x509_dir, *addr, +char *password = NULL; +const char *passwordSecret; +const char *str, *x509_dir, *addr, *x509_key_password = NULL, *x509_dh_file = NULL, *tls_ciphers = NULL; @@ -663,7 +669,24 @@ static void qemu_spice_init(void) error_report("spice tls-port is out of range"); exit(1); } -password = qemu_opt_get(opts, "password"); +passwordSecret = qemu_opt_get(opts, "password-secret"); +if (passwordSecret) { +Error *local_err = NULL; +if (qemu_opt_get(opts, "password")) { +error_report("'password' option is mutually exclusive with " + "'password-secret'"); +exit(1); +} +password = qcrypto_secret_lookup_as_utf8(passwordSecret, + _err); +if (!password) { +error_report_err(local_err); +exit(1); +} +} else { +str = qemu_opt_get(opts, "password"); +password = g_strdup(str); +} if (tls_port) { x509_dir = qemu_opt_get(opts, "x509-dir"); @@ -809,6 +832,7 @@ static void qemu_spice_init(void) g_free(x509_key_file); g_free(x509_cert_file); g_free(x509_cacert_file); +g_free(password); #ifdef HAVE_SPICE_GL if (qemu_opt_get_bool(opts, "gl", 0)) { -- 2.29.2
[PATCH 4/4] ui, monitor: remove deprecated VNC ACL option and HMP commands
The VNC ACL concept has been replaced by the pluggable "authz" framework which does not use monitor commands. Signed-off-by: Daniel P. Berrangé --- docs/system/deprecated.rst | 16 --- docs/system/removed-features.rst | 13 +++ hmp-commands.hx | 76 - monitor/misc.c | 187 --- ui/vnc.c | 38 --- 5 files changed, 13 insertions(+), 317 deletions(-) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 57ff9f47cc..beed4b4f02 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -37,12 +37,6 @@ The 'file' driver for drives is no longer appropriate for character or host devices and will only accept regular files (S_IFREG). The correct driver for these file types is 'host_cdrom' or 'host_device' as appropriate. -``-vnc acl`` (since 4.0.0) -'' - -The ``acl`` option to the ``-vnc`` argument has been replaced -by the ``tls-authz`` and ``sasl-authz`` options. - ``QEMU_AUDIO_`` environment variables and ``-audio-help`` (since 4.0) ' @@ -262,16 +256,6 @@ Use the more generic commands ``block-export-add`` and ``block-export-del`` instead. As part of this deprecation, where ``nbd-server-add`` used a single ``bitmap``, the new ``block-export-add`` uses a list of ``bitmaps``. -Human Monitor Protocol (HMP) commands -- - -``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, ``acl_remove`` (since 4.0.0) -'' - -The ``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, and -``acl_remove`` commands are deprecated with no replacement. Authorization -for VNC should be performed using the pluggable QAuthZ objects. - System emulator CPUS diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst index c8481cafbd..0424b9a89d 100644 --- a/docs/system/removed-features.rst +++ b/docs/system/removed-features.rst @@ -38,6 +38,12 @@ or ``-display default,show-cursor=on`` instead. QEMU 5.0 introduced an alternative syntax to specify the size of the translation block cache, ``-accel tcg,tb-size=``. +``-vnc acl`` (removed in 6.0) +' + +The ``acl`` option to the ``-vnc`` argument has been replaced +by the ``tls-authz`` and ``sasl-authz`` options. + QEMU Machine Protocol (QMP) commands @@ -79,6 +85,13 @@ documentation of ``query-hotpluggable-cpus`` for additional details. No replacement. The ``change vnc password`` and ``change DEVICE MEDIUM`` commands are not affected. +``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, ``acl_remove`` (removed in 6.0) +' + +The ``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, and +``acl_remove`` commands were removed with no replacement. Authorization +for VNC should be performed using the pluggable QAuthZ objects. + Guest Emulator ISAs --- diff --git a/hmp-commands.hx b/hmp-commands.hx index d4001f9c5d..b500b8526d 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1433,82 +1433,6 @@ SRST Change watchdog action. ERST -{ -.name = "acl_show", -.args_type = "aclname:s", -.params = "aclname", -.help = "list rules in the access control list", -.cmd= hmp_acl_show, -}, - -SRST -``acl_show`` *aclname* - List all the matching rules in the access control list, and the default - policy. There are currently two named access control lists, - *vnc.x509dname* and *vnc.username* matching on the x509 client - certificate distinguished name, and SASL username respectively. -ERST - -{ -.name = "acl_policy", -.args_type = "aclname:s,policy:s", -.params = "aclname allow|deny", -.help = "set default access control list policy", -.cmd= hmp_acl_policy, -}, - -SRST -``acl_policy`` *aclname* ``allow|deny`` - Set the default access control list policy, used in the event that - none of the explicit rules match. The default policy at startup is - always ``deny``. -ERST - -{ -.name = "acl_add", -.args_type = "aclname:s,match:s,policy:s,index:i?", -.params = "aclname match allow|deny [index]", -.help = "add a match rule to the access control list", -.cmd= hmp_acl_add, -}, - -SRST -``acl_add`` *aclname* *match* ``allow|deny`` [*index*] - Add a match rule to the access control list, allowing or denying access. - The match will normally be an exact username or x509 distinguished name, - but can optionally include wildcard globs. eg ``*@EXAMPLE.COM`` to - allow all users
[PATCH 1/4] ui: introduce "password-secret" option for VNC servers
Currently when using VNC the "password" flag turns on password based authentication. The actual password has to be provided separately via the monitor. This introduces a "password-secret" option which lets the password be provided up front. $QEMU --object secret,id=vncsec0,file=passwd.txt \ --vnc localhost:0,password-secret=vncsec0 Signed-off-by: Daniel P. Berrangé --- qemu-options.hx | 5 + ui/vnc.c| 23 ++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/qemu-options.hx b/qemu-options.hx index 6c34c7050f..893d0f500b 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2164,6 +2164,11 @@ SRST time to allow password to expire immediately or never expire. +``password-secret=`` +Require that password based authentication is used for client +connections, using the password provided by the ``secret`` +object identified by ``secret-id``. + ``tls-creds=ID`` Provides the ID of a set of TLS credentials to use to secure the VNC server. They will apply to both the normal VNC server socket diff --git a/ui/vnc.c b/ui/vnc.c index 16bb3be770..77e07ac351 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -48,6 +48,7 @@ #include "crypto/tlscredsanon.h" #include "crypto/tlscredsx509.h" #include "crypto/random.h" +#include "crypto/secret_common.h" #include "qom/object_interfaces.h" #include "qemu/cutils.h" #include "qemu/help_option.h" @@ -3469,6 +3470,9 @@ static QemuOptsList qemu_vnc_opts = { },{ .name = "password", .type = QEMU_OPT_BOOL, +},{ +.name = "password-secret", +.type = QEMU_OPT_STRING, },{ .name = "reverse", .type = QEMU_OPT_BOOL, @@ -3941,6 +3945,7 @@ void vnc_display_open(const char *id, Error **errp) int lock_key_sync = 1; int key_delay_ms; const char *audiodev; +const char *passwordSecret; if (!vd) { error_setg(errp, "VNC display not active"); @@ -3958,7 +3963,23 @@ void vnc_display_open(const char *id, Error **errp) goto fail; } -password = qemu_opt_get_bool(opts, "password", false); + +passwordSecret = qemu_opt_get(opts, "password-secret"); +if (passwordSecret) { +if (qemu_opt_get(opts, "password")) { +error_setg(errp, + "'password' flag is redundant with 'password-secret'"); +goto fail; +} +vd->password = qcrypto_secret_lookup_as_utf8(passwordSecret, + errp); +if (!vd->password) { +goto fail; +} +password = true; +} else { +password = qemu_opt_get_bool(opts, "password", false); +} if (password) { if (fips_get_state()) { error_setg(errp, -- 2.29.2
[PATCH 3/4] ui: deprecate "password" option for SPICE server
With the new "password-secret" option, there is no reason to use the old inecure "password" option with -spice, so it can be deprecated. Signed-off-by: Daniel P. Berrangé --- docs/system/deprecated.rst | 8 qemu-options.hx| 4 ui/spice-core.c| 4 3 files changed, 16 insertions(+) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 2fcac7861e..57ff9f47cc 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -146,6 +146,14 @@ library enabled as a cryptography provider. Neither the ``nettle`` library, or the built-in cryptography provider are supported on FIPS enabled hosts. +``-spice password=string`` (since 6.0) +'' + +This option is insecure because the SPICE password remains visible in +the process listing. This is replaced by the new ``password-secret`` +option which lets the password be securely provided on the command +line using a ``secret`` object instance. + QEMU Machine Protocol (QMP) commands diff --git a/qemu-options.hx b/qemu-options.hx index ff4ef3b708..4833bd59cf 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1926,6 +1926,10 @@ SRST ``password=`` Set the password you need to authenticate. +This option is deprecated and insecure because it leaves the +password visible in the process listing. Use ``password-secret`` +instead. + ``password-secret=`` Set the ID of the ``secret`` object containing the password you need to authenticate. diff --git a/ui/spice-core.c b/ui/spice-core.c index 353848b244..5e00e31457 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -685,6 +685,10 @@ static void qemu_spice_init(void) } } else { str = qemu_opt_get(opts, "password"); +if (str) { +warn_report("'password' option is deprecated and insecure, " +"use 'password-secret' instead"); +} password = g_strdup(str); } -- 2.29.2
[PATCH 0/4] ui: add support for 'secret' object to provide VNC/SPICE passwords
This fixes a long standing limitation of the VNC/SPICE code which was unable to securely accept passswords on the CLI, instead requiring use of separate monitor commands after startup. This takes the opportunity to also remove previously deprecated ACL functionality from VNC. Daniel P. Berrangé (4): ui: introduce "password-secret" option for VNC servers ui: introduce "password-secret" option for SPICE server ui: deprecate "password" option for SPICE server ui, monitor: remove deprecated VNC ACL option and HMP commands docs/system/deprecated.rst | 24 ++-- docs/system/removed-features.rst | 13 +++ hmp-commands.hx | 76 - monitor/misc.c | 187 --- qemu-options.hx | 17 ++- ui/spice-core.c | 32 +- ui/vnc.c | 61 -- 7 files changed, 88 insertions(+), 322 deletions(-) -- 2.29.2
link to User documentation of https://wiki.qemu.org/Features/Tracing is broken currently
Hi all, the link to User documentation of https://wiki.qemu.org/Features/Tracing is broken currently: it points to: http://git.qemu-project.org/?p=qemu.git;a=blob_plain;f=docs/devel/tracing.txt;hb=HEAD and that to me gives a 404 - Cannot find file. Ciao, Claudio -- Claudio Fontana Engineering Manager Virtualization, SUSE Labs Core SUSE Software Solutions Italy Srl
Re: FreeBSD build regressions
On Fri, Feb 19, 2021 at 9:14 AM Peter Maydell wrote: > On Fri, 19 Feb 2021 at 16:08, Warner Losh wrote: > > FreeBSD builds packages on the oldest supported version in the stable > branch. Due to forward compatibility, that means all supported versions of > FreeBSD 12.x will work. Recently, FreeBSD 12.1 became unsupported, so the > build machines clicked forward to 12.2. Since there's no 'forward > compatibility' guarantees, this problem was hit. While you can run binaries > compiled on old versions of the software on new versions of the system, you > can't necessarily do the inverse because new symbols are introduced (in > this case close_range). > > It makes perfect sense that you don't want to support older > versions forever and that at some point newer packages aren't > valid on old systems, but I don't understand why an > older 12.1 system then says "but I'm going to go ahead and > install these won't-work packages anyway" rather than > "oh dear, I'm out of support, there are no newer packages > available, I will install whatever the last archived version > of the package for my OS version is" (or "I will install nothing"). > I'm surprised this doesn't break a lot of real-world users... > That's a reasonable expectation. I'd kinda expected that to be the default, but it looks like it might not be. I'll see if I can get the freebsd vm updated to use something safer and/or work with the pkg folks to get it to do the safe thing here if there's no easy way to do this with command line / config settings. I think the issue is that we set IGNORE_OSVERSION which is needed for the case when we were running 12.0 packages on 12.1, but it's harmful for this case. This highlights, I think, a rough edge in pkg. Short term, I'll bump things up to 12.2 which will take care of the immediate issue. I should have a patch by later in the day I may also have a patch to detect the mismatch directly and report it until this issue can be resolved in FreeBSD's pkg. Warner
Re: who's using the ozlabs patchwork install for QEMU patches ?
On 2/19/21 7:07 PM, BALATON Zoltan wrote: > On Fri, 19 Feb 2021, Peter Maydell wrote: >> Does anybody use the ozlabs patchwork install for QEMU patches, >> either occasionally or on a regular basis ? >> http://patchwork.ozlabs.org/project/qemu-devel/list/ >> The admins for that system are trying to identify which of >> the various projects are really using their patchwork instances, >> so I figured I'd do a quick survey here. We don't use it >> as an official project tool but it's certainly possible to >> use it as an individual developer in one way or another. > > The "How to submit a patch" page at > https://wiki.qemu.org/Contribute/SubmitAPatch#If_your_patch_seems_to_have_been_ignored > > says to send patchew URL with pings. Does that make it "official"? Thanks for the reminder, I updated the patchwork URL to patchew :)