[Bug 1916112] Re: Illegal instruction crash of QEMU on Jetson Nano

2021-02-19 Thread Ravishankar
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

2021-02-19 Thread Bin Meng
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

2021-02-19 Thread Howard Spoelstra
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

2021-02-19 Thread Huacai Chen
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-02-19 Thread Akihiko Odaki
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

2021-02-19 Thread Frank Chang
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-02-19 Thread Jiaxun Yang

在 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

2021-02-19 Thread Akihiko Odaki
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

2021-02-19 Thread Launchpad Bug Tracker
[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

2021-02-19 Thread Bin Meng
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

2021-02-19 Thread Alexander Bulekov
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

2021-02-19 Thread Alexander Bulekov
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

2021-02-19 Thread Bin Meng
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

2021-02-19 Thread Bin Meng
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

2021-02-19 Thread Akihiko Odaki
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

2021-02-19 Thread no-reply
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

2021-02-19 Thread Philippe Mathieu-Daudé
** 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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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()

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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()

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Doug Evans via
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Doug Evans via
... 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

2021-02-19 Thread Doug Evans via
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)

2021-02-19 Thread Doug Evans via
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

2021-02-19 Thread Doug Evans via
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Doug Evans via
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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()

2021-02-19 Thread Philippe Mathieu-Daudé
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()

2021-02-19 Thread Philippe Mathieu-Daudé
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_

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Peter Xu
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

2021-02-19 Thread Peter Maydell
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Peter Maydell
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()

2021-02-19 Thread Peter Maydell
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

2021-02-19 Thread Peter Maydell
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_

2021-02-19 Thread Peter Maydell
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Peter Maydell
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

2021-02-19 Thread Doug Evans
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Philippe Mathieu-Daudé
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

2021-02-19 Thread Gerd Hoffmann
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

2021-02-19 Thread Gerd Hoffmann
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

2021-02-19 Thread Gerd Hoffmann
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

2021-02-19 Thread Gerd Hoffmann
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

2021-02-19 Thread Gerd Hoffmann
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

2021-02-19 Thread Cleber Rosa
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

2021-02-19 Thread Gerd Hoffmann
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

2021-02-19 Thread Gerd Hoffmann
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

2021-02-19 Thread Gerd Hoffmann
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

2021-02-19 Thread Cleber Rosa
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

2021-02-19 Thread Gerd Hoffmann
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)

2021-02-19 Thread Cleber Rosa
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

2021-02-19 Thread Cleber Rosa
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

2021-02-19 Thread Cleber Rosa
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

2021-02-19 Thread Doug Evans
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

2021-02-19 Thread Warner Losh
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

2021-02-19 Thread Warner Losh
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

2021-02-19 Thread Daniel Henrique Barboza




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

2021-02-19 Thread David Hildenbrand


> 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

2021-02-19 Thread 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...)


> -- 
> Peter Xu
> 


Re: [PATCH v13 0/5] UFFD write-tracking migration/snapshots

2021-02-19 Thread 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..

-- 
Peter Xu




Re: [PATCH v13 0/5] UFFD write-tracking migration/snapshots

2021-02-19 Thread Peter Xu
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

2021-02-19 Thread no-reply
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

2021-02-19 Thread Connor Kuehl

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

2021-02-19 Thread Peter Collingbourne via
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

2021-02-19 Thread Daniel Henrique Barboza




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)

2021-02-19 Thread Vivek Goyal
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

2021-02-19 Thread Cleber Rosa
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

2021-02-19 Thread Pierre Morel




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

2021-02-19 Thread Daniel P . Berrangé
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

2021-02-19 Thread Daniel P . Berrangé
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

2021-02-19 Thread Daniel P . Berrangé
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

2021-02-19 Thread Daniel P . Berrangé
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

2021-02-19 Thread Daniel P . Berrangé
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

2021-02-19 Thread Claudio Fontana
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

2021-02-19 Thread Warner Losh
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 ?

2021-02-19 Thread Philippe Mathieu-Daudé
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 :)



  1   2   3   4   >