date:20210301

On 2/25/21 8:47 PM, BALATON Zoltan wrote:
> The Marvell Discovery II aka. MV64361 is a PowerPC system controller
> chip that is used on the pegasos2 PPC board. This adds emulation of it
> that models the device enough to boot guests on this board. The
> mv643xx.h header with register definitions is taken from Linux 4.15.10
> only fixing end of line white space errors and removing not needed
> parts, it's otherwise keeps Linux formatting.
> 
> Signed-off-by: BALATON Zoltan 
> ---
>  hw/pci-host/Kconfig   |   3 +
>  hw/pci-host/meson.build   |   2 +
>  hw/pci-host/mv64361.c | 966 ++
>  hw/pci-host/mv643xx.h | 919 

Maybe name this one mv643xx_regs.h?

>  hw/pci-host/trace-events  |   6 +
>  include/hw/pci-host/mv64361.h |   8 +
>  include/hw/pci/pci_ids.h  |   1 +
>  7 files changed, 1905 insertions(+)
>  create mode 100644 hw/pci-host/mv64361.c
>  create mode 100644 hw/pci-host/mv643xx.h
>  create mode 100644 include/hw/pci-host/mv64361.h

> 
> diff --git a/hw/pci-host/Kconfig b/hw/pci-host/Kconfig
> index 8b8c763c28..65a983d6fd 100644
> --- a/hw/pci-host/Kconfig
> +++ b/hw/pci-host/Kconfig
> @@ -68,3 +68,6 @@ config PCI_POWERNV
>  
>  config REMOTE_PCIHOST
>  bool
> +
> +config MV64361
> +bool

Missing:

   select PCI

Re: [RFC v2 10/24] target/arm: only perform TCG cpu and machine inits if tcg enabled

2021-03-01 Thread Claudio Fontana

On 3/2/21 4:36 AM, Richard Henderson wrote:
> On 3/1/21 8:49 AM, Claudio Fontana wrote:
>> @@ -1321,6 +1323,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
>> **errp)
>>   }
>>   }
>>   
>> +#ifdef CONFIG_TCG
>>   {
>>   uint64_t scale;
>>   
>> @@ -1346,7 +1349,8 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
>> **errp)
>>   cpu->gt_timer[GTIMER_HYPVIRT] = timer_new(QEMU_CLOCK_VIRTUAL, 
>> scale,
>> arm_gt_hvtimer_cb, cpu);
>>   }
>> -#endif
>> +#endif /* CONFIG_TCG */
> 
> You can use tcg_enabled here.

It is not possible currently. I probably could write a comment about it.

It would break migration for the --enable-kvm --enable-tcg build, due to

+#ifdef CONFIG_TCG
 VMSTATE_TIMER_PTR(gt_timer[GTIMER_PHYS], ARMCPU),
 VMSTATE_TIMER_PTR(gt_timer[GTIMER_VIRT], ARMCPU),
+#else
+VMSTATE_UNUSED(sizeof(QEMUTimer *)),
+VMSTATE_UNUSED(sizeof(QEMUTimer *)),
+#endif /* CONFIG_TCG */



> 
>> -}
>>   
>>   #ifndef CONFIG_USER_ONLY
>> -cpu->pmu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, arm_pmu_timer_cb,
>> -cpu);
>> +cpu->pmu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, 
>> arm_pmu_timer_cb,
>> +  cpu);
> 
> Incorrect indentation change.
> Otherwise, LGTM.
> 
> 
> r~
> 

you need more lines of context in this patch than what you are quoting here,
the indentation change is intended.

The whole code, including cpu->pmu_timer is now only executed under 
tcg_enabled().

Thanks,

CLaudio

Re: [PATCH v4 3/6] vt82c686: Add VT8231_SUPERIO based on VIA_SUPERIO

On 2/25/21 8:47 PM, BALATON Zoltan wrote:
> The VT8231 south bridge is very similar to VT82C686B but there are
> some differences in register addresses and functionality, e.g. the
> VT8231 only has one serial port. This commit adds VT8231_SUPERIO
> subclass based on the abstract VIA_SUPERIO class to emulate the
> superio part of VT8231.
> 
> Signed-off-by: BALATON Zoltan 
> ---
>  hw/isa/vt82c686.c | 121 ++
>  1 file changed, 121 insertions(+)
> 
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 9b2ffecc79..72234bc4d1 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -489,6 +489,126 @@ static const TypeInfo vt82c686b_superio_info = {
>  };
>  
>  
> +#define TYPE_VT8231_SUPERIO "vt8231-superio"
> +
> +static void vt8231_superio_cfg_write(void *opaque, hwaddr addr,
> + uint64_t data, unsigned size)
> +{
> +ViaSuperIOState *sc = opaque;
> +uint8_t idx = sc->regs[0];
> +
> +if (addr == 0) { /* config index register */
> +sc->regs[0] = data;
> +return;
> +}
> +
> +/* config data register */
> +trace_via_superio_write(idx, data);
> +switch (idx) {
> +case 0x00 ... 0xdf:
> +case 0xe7 ... 0xef:
> +case 0xf0 ... 0xf1:
> +case 0xf5:
> +case 0xf8:
> +case 0xfd:
> +/* ignore write to read only registers */
> +return;
> +case 0xf2: /* Function select */
> +{
> +data &= 0x17;

I'd prefer a definition for this mask (and @0xf4) but well...
Reviewed-by: Philippe Mathieu-Daudé 

> +if (data & BIT(2)) { /* Serial port enable */
> +ISADevice *dev = sc->superio.serial[0];
> +if (!memory_region_is_mapped(sc->serial_io[0])) {
> +memory_region_add_subregion(isa_address_space_io(dev),
> +dev->ioport_id, 
> sc->serial_io[0]);
> +}
> +} else {
> +MemoryRegion *io = isa_address_space_io(sc->superio.serial[0]);
> +if (memory_region_is_mapped(sc->serial_io[0])) {
> +memory_region_del_subregion(io, sc->serial_io[0]);
> +}
> +}
> +break;
> +}
> +case 0xf4: /* Serial port io base address */
> +{
> +data &= 0xfe;
> +sc->superio.serial[0]->ioport_id = data << 2;
> +if (memory_region_is_mapped(sc->serial_io[0])) {
> +memory_region_set_address(sc->serial_io[0], data << 2);
> +}
> +break;
> +}
> +default:
> +qemu_log_mask(LOG_UNIMP,
> +  "via_superio_cfg: unimplemented register 0x%x\n", idx);
> +break;
> +}
> +sc->regs[idx] = data;
> +}

Re: [PATCH v4 4/6] vt82c686: Add emulation of VT8231 south bridge

On 2/25/21 8:47 PM, BALATON Zoltan wrote:
> Add emulation of VT8231 south bridge ISA part based on the similar
> VT82C686B but implemented in a separate subclass that holds the
> differences while reusing parts that can be shared.
> 
> Signed-off-by: BALATON Zoltan 
> ---
>  hw/isa/vt82c686.c | 154 ++
>  include/hw/isa/vt82c686.h |   1 +
>  include/hw/pci/pci_ids.h  |   3 +-
>  3 files changed, 126 insertions(+), 32 deletions(-)
> 
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 72234bc4d1..e0f2f2a5ce 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -8,6 +8,9 @@
>   *
>   * Contributions after 2012-01-13 are licensed under the terms of the
>   * GNU GPL, version 2 or (at your option) any later version.
> + *
> + * VT8231 south bridge support and general clean up to allow it
> + * Copyright (c) 2018-2020 BALATON Zoltan
>   */
>  
>  #include "qemu/osdep.h"
> @@ -609,24 +612,48 @@ static const TypeInfo vt8231_superio_info = {
>  };
>  
>  
> -OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BISAState, VT82C686B_ISA)
> +#define TYPE_VIA_ISA "via-isa"
> +OBJECT_DECLARE_SIMPLE_TYPE(ViaISAState, VIA_ISA)
>  
> -struct VT82C686BISAState {
> +struct ViaISAState {
>  PCIDevice dev;
>  qemu_irq cpu_intr;
>  ViaSuperIOState *via_sio;
>  };
>  
> +static const VMStateDescription vmstate_via = {
> +.name = "via-isa",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_PCI_DEVICE(dev, ViaISAState),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> +static const TypeInfo via_isa_info = {
> +.name  = TYPE_VIA_ISA,
> +.parent= TYPE_PCI_DEVICE,
> +.instance_size = sizeof(ViaISAState),
> +.abstract  = true,
> +.interfaces= (InterfaceInfo[]) {
> +{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
> +{ },
> +},
> +};
> +
>  static void via_isa_request_i8259_irq(void *opaque, int irq, int level)
>  {
> -VT82C686BISAState *s = opaque;
> +ViaISAState *s = opaque;
>  qemu_set_irq(s->cpu_intr, level);
>  }
>  
> +/* TYPE_VT82C686B_ISA */
> +
>  static void vt82c686b_write_config(PCIDevice *d, uint32_t addr,
> uint32_t val, int len)
>  {
> -VT82C686BISAState *s = VT82C686B_ISA(d);
> +ViaISAState *s = VIA_ISA(d);
>  
>  trace_via_isa_write(addr, val, len);
>  pci_default_write_config(d, addr, val, len);
> @@ -636,19 +663,9 @@ static void vt82c686b_write_config(PCIDevice *d, 
> uint32_t addr,
>  }
>  }
>  
> -static const VMStateDescription vmstate_via = {
> -.name = "vt82c686b",
> -.version_id = 1,
> -.minimum_version_id = 1,
> -.fields = (VMStateField[]) {
> -VMSTATE_PCI_DEVICE(dev, VT82C686BISAState),
> -VMSTATE_END_OF_LIST()
> -}
> -};
> -
>  static void vt82c686b_isa_reset(DeviceState *dev)
>  {
> -VT82C686BISAState *s = VT82C686B_ISA(dev);
> +ViaISAState *s = VIA_ISA(dev);
>  uint8_t *pci_conf = s->dev.config;
>  
>  pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x00c0);
> @@ -668,7 +685,7 @@ static void vt82c686b_isa_reset(DeviceState *dev)
>  
>  static void vt82c686b_realize(PCIDevice *d, Error **errp)
>  {
> -VT82C686BISAState *s = VT82C686B_ISA(d);
> +ViaISAState *s = VIA_ISA(d);
>  DeviceState *dev = DEVICE(d);
>  ISABus *isa_bus;
>  qemu_irq *isa_irq;
> @@ -692,7 +709,7 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
>  }
>  }
>  
> -static void via_class_init(ObjectClass *klass, void *data)
> +static void vt82c686b_class_init(ObjectClass *klass, void *data)
>  {
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> @@ -700,28 +717,101 @@ static void via_class_init(ObjectClass *klass, void 
> *data)
>  k->realize = vt82c686b_realize;
>  k->config_write = vt82c686b_write_config;
>  k->vendor_id = PCI_VENDOR_ID_VIA;
> -k->device_id = PCI_DEVICE_ID_VIA_ISA_BRIDGE;
> +k->device_id = PCI_DEVICE_ID_VIA_82C686B_ISA;
>  k->class_id = PCI_CLASS_BRIDGE_ISA;
>  k->revision = 0x40;
>  dc->reset = vt82c686b_isa_reset;
>  dc->desc = "ISA bridge";
>  dc->vmsd = _via;
> -/*
> - * Reason: part of VIA VT82C686 southbridge, needs to be wired up,
> - * e.g. by mips_fuloong2e_init()
> - */
> +/* Reason: part of VIA VT82C686 southbridge, needs to be wired up */
>  dc->user_creatable = false;
>  }
>  
> -static const TypeInfo via_info = {
> +static const TypeInfo vt82c686b_isa_info = {
>  .name  = TYPE_VT82C686B_ISA,
> -.parent= TYPE_PCI_DEVICE,
> -.instance_size = sizeof(VT82C686BISAState),
> -.class_init= via_class_init,
> -.interfaces = (InterfaceInfo[]) {
> -{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
> -{ },
> -},
> +.parent= TYPE_VIA_ISA,
> +.instance_size = sizeof(ViaISAState),
> +.class_init=

Re: [RFC v2 24/24] target/arm: wrap call to aarch64_sve_change_el in tcg_enabled()

2021-03-01 Thread Claudio Fontana

Probably I should describe here that after this patch,

configure --enable-tcg --disable-kvm

is now buildable.

Ciao,

Claudio

On 3/1/21 5:49 PM, Claudio Fontana wrote:
> Signed-off-by: Claudio Fontana 
> ---
>  target/arm/cpu-sysemu.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/target/arm/cpu-sysemu.c b/target/arm/cpu-sysemu.c
> index 451ad4f8bd..034d073c38 100644
> --- a/target/arm/cpu-sysemu.c
> +++ b/target/arm/cpu-sysemu.c
> @@ -754,11 +754,13 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
>  unsigned int cur_el = arm_current_el(env);
>  int rt;
>  
> -/*
> - * Note that new_el can never be 0.  If cur_el is 0, then
> - * el0_a64 is is_a64(), else el0_a64 is ignored.
> - */
> -aarch64_sve_change_el(env, cur_el, new_el, is_a64(env));
> +if (tcg_enabled()) {
> +/*
> + * Note that new_el can never be 0.  If cur_el is 0, then
> + * el0_a64 is is_a64(), else el0_a64 is ignored.
> + */
> +aarch64_sve_change_el(env, cur_el, new_el, is_a64(env));
> +}
>  
>  if (cur_el < new_el) {
>  /* Entry vector offset depends on whether the implemented EL
>

Re: [RFC PATCH v2 17/17] cpu: Restrict "hw/core/sysemu-cpu-ops.h" to target/cpu.c

On 3/1/21 10:51 PM, Philippe Mathieu-Daudé wrote:
> Somehow similar to commit 78271684719 ("cpu: tcg_ops: move to
> tcg-cpu-ops.h, keep a pointer in CPUClass"):
> 
> We cannot in principle make the SysEmu Operations field definitions
> conditional on CONFIG_SOFTMMU in code that is included by both
> common_ss and specific_ss modules.
> 
> Therefore, what we can do safely to restrict the SysEmu fields to
> system emulation builds, is to move all sysemu operations into a
> separate header file, which is only included by system-specific code.
> 
> This leaves just a NULL pointer in the cpu.h for the user-mode builds.
> 
> Inspired-by: Claudio Fontana 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> RFC: improve commit description?
> 
>  include/hw/core/cpu.h   | 3 ++-
>  cpu.c   | 1 +
>  hw/core/cpu.c   | 1 +
>  target/alpha/cpu.c  | 1 +
>  target/arm/cpu.c| 1 +
>  target/avr/cpu.c| 1 +
>  target/cris/cpu.c   | 1 +
>  target/hppa/cpu.c   | 1 +
>  target/i386/cpu.c   | 1 +
>  target/m68k/cpu.c   | 1 +
>  target/microblaze/cpu.c | 1 +
>  target/mips/cpu.c   | 1 +
>  target/moxie/cpu.c  | 1 +
>  target/nios2/cpu.c  | 1 +
>  target/openrisc/cpu.c   | 1 +
>  target/riscv/cpu.c  | 1 +
>  target/rx/cpu.c | 1 +
>  target/s390x/cpu.c  | 1 +
>  target/sh4/cpu.c| 1 +
>  target/sparc/cpu.c  | 1 +
>  target/tricore/cpu.c| 1 +
>  target/xtensa/cpu.c | 1 +
>  target/ppc/translate_init.c.inc | 1 +
>  23 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index d99d3c830dc..398696f0f2d 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -80,7 +80,8 @@ struct TCGCPUOps;
>  /* see accel-cpu.h */
>  struct AccelCPUClass;
>  
> -#include "hw/core/sysemu-cpu-ops.h"
> +/* see sysemu-cpu-ops.h */
> +struct SysemuCPUOps;

As the deprecated targets aren't built by default, I missed:

-- >8 --
diff --git a/target/lm32/cpu.c b/target/lm32/cpu.c
index c80cae9ff3b..eed7f3f440c 100644
--- a/target/lm32/cpu.c
+++ b/target/lm32/cpu.c
@@ -22,6 +22,7 @@
 #include "qapi/error.h"
 #include "qemu/qemu-print.h"
 #include "cpu.h"
+#include "hw/core/sysemu-cpu-ops.h"


 static void lm32_cpu_set_pc(CPUState *cs, vaddr value)
diff --git a/target/unicore32/cpu.c b/target/unicore32/cpu.c
index 610fb5393ae..afe106da2d4 100644
--- a/target/unicore32/cpu.c
+++ b/target/unicore32/cpu.c
@@ -17,6 +17,7 @@
 #include "cpu.h"
 #include "migration/vmstate.h"
 #include "exec/exec-all.h"
+#include "hw/core/sysemu-cpu-ops.h"

 static void uc32_cpu_set_pc(CPUState *cs, vaddr value)
 {
---

Re: [Bug 1917394] [NEW] command lspci does not show the IVSHMEM device

2021-03-01 Thread ChangLimin

Can you give the lspci messages? The below is my output.  There is a RAM memory 
device.

$ lspci
00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02)
00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II]
00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II]
00:01.2 USB controller: Intel Corporation 82371SB PIIX3 USB [Natoma/Triton II] 
(rev 01)
00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03)
00:02.0 VGA compatible controller: Device 1234: (rev 02)
00:03.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge
00:04.0 Ethernet controller: Red Hat, Inc. Virtio network device
00:05.0 SCSI storage controller: Red Hat, Inc. Virtio SCSI
00:06.0 Communication controller: Red Hat, Inc. Virtio console
00:10.0 RAM memory: Red Hat, Inc. Inter-VM shared memory (rev 01)
01:07.0 PCI bridge: Red Hat, Inc. QEMU PCI-PCI bridge

 
From: sean kuo
Date: 2021-03-02 11:24
To: qemu-devel
Subject: [Bug 1917394] [NEW] command lspci does not show the IVSHMEM device
Public bug reported:
 
qeum version:
QEMU emulator version 4.2.1
 
I met a problem when I tried to use IVSHMEM. Command lspci does not show the 
IVSHMEM device.
Below is the configuration from my side:
 
1.  guest vm xml configuration.
  
  
  2
  

 
2. after the booting up and I found the qemu commandline ideedly  have the 
device option:
ps aux | grep ivshmem
/usr/bin/qemu-system-x86_64 
  ...(ignore other options)
-object 
memory-backend-file,id=shmmem-shmem0,mem-path=/dev/shm/hostmem,size=4194304,share=yes
 -device ivshmem-plain,id=shmem0,memdev=shmmem-shmem0,bus=pcie.0,addr=0x10
 
3. lspci command  not shown the device.
 
4. lshw command indeedly show the device:
 
*-memory UNCLAIMED
 description: RAM memory
 product: Inter-VM shared memory
 vendor: Red Hat, Inc.
 physical id: 10
 bus info: pci@:00:10.0
 version: 01
 width: 64 bits
 clock: 33MHz (30.3ns)
 configuration: latency=0
 resources: memory:fcc1c000-fcc1c0ff memory:fdc0-fdff
 
My host and vm os is ubuntu 20.04 and version is:
#49~20.04.1-Ubuntu SMP Fri Feb 5 09:57:56 UTC 2021 x86_64 x86_64 x86_64 
GNU/Linux
 
** Affects: qemu
 Importance: Undecided
 Status: New
 
-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1917394
 
Title:
  command lspci does not show the IVSHMEM device
 
Status in QEMU:
  New
 
Bug description:
  qeum version:
  QEMU emulator version 4.2.1
 
  I met a problem when I tried to use IVSHMEM. Command lspci does not show the 
IVSHMEM device.
  Below is the configuration from my side:
 
  1.  guest vm xml configuration.


2

  
 
  2. after the booting up and I found the qemu commandline ideedly  have the 
device option:
  ps aux | grep ivshmem
   /usr/bin/qemu-system-x86_64 
...(ignore other options)
  -object 
memory-backend-file,id=shmmem-shmem0,mem-path=/dev/shm/hostmem,size=4194304,share=yes
 -device ivshmem-plain,id=shmem0,memdev=shmmem-shmem0,bus=pcie.0,addr=0x10
 
  3. lspci command  not shown the device.
 
  4. lshw command indeedly show the device:
 
  *-memory UNCLAIMED
   description: RAM memory
   product: Inter-VM shared memory
   vendor: Red Hat, Inc.
   physical id: 10
   bus info: pci@:00:10.0
   version: 01
   width: 64 bits
   clock: 33MHz (30.3ns)
   configuration: latency=0
   resources: memory:fcc1c000-fcc1c0ff memory:fdc0-fdff
 
  My host and vm os is ubuntu 20.04 and version is:
  #49~20.04.1-Ubuntu SMP Fri Feb 5 09:57:56 UTC 2021 x86_64 x86_64 x86_64 
GNU/Linux
 
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1917394/+subscriptions

Re: [PATCH qemu v14] spapr: Implement Open Firmware client interface

2021-03-01 Thread Alexey Kardashevskiy

On 02/03/2021 14:35, David Gibson wrote:

On Wed, Feb 24, 2021 at 04:41:30PM +1100, Alexey Kardashevskiy wrote:

The PAPR platform which describes an OS environment that's presented by
a combination of a hypervisor and firmware. The features it specifies
require collaboration between the firmware and the hypervisor.

Since the beginning, the runtime component of the firmware (RTAS) has
been implemented as a 20 byte shim which simply forwards it to
a hypercall implemented in qemu. The boot time firmware component is
SLOF - but a build that's specific to qemu, and has always needed to be
updated in sync with it. Even though we've managed to limit the amount
of runtime communication we need between qemu and SLOF, there's some,
and it has become increasingly awkward to handle as we've implemented
new features.

This implements a boot time OF client interface (CI) which is
enabled by a new "x-vof" pseries machine option (stands for "Virtual Open
Firmware). When enabled, QEMU implements the custom H_OF_CLIENT hcall
which implements Open Firmware Client Interface (OF CI). This allows
using a smaller stateless firmware which does not have to manage
the device tree.

The new "vof.bin" firmware image is included with source code under
pc-bios/. It also includes RTAS blob.

This implements a handful of CI methods just to get -kernel/-initrd
working. In particular, this implements the device tree fetching and
simple memory allocator - "claim" (an OF CI memory allocator) and updates
"/memory@0/available" to report the client about available memory.

This implements changing some device tree properties which we know how
to deal with, the rest is ignored. To allow changes, this skips
fdt_pack() when x-vof=on as not packing the blob leaves some room for
appending.

In absence of SLOF, this assigns phandles to device tree nodes to make
device tree traversing work.

When x-vof=on, this adds "/chosen" every time QEMU (re)builds a tree.

This adds basic instances support which are managed by a hash map
ihandle -> [phandle].

Before the guest started, the used memory is:
0..4000 - the initial firmware
1..18 - stack

This OF CI does not implement "interpret".

Unlike SLOF, this does not format uninitialized nvram. Instead, this
includes a disk image with pre-formatted nvram.

I think we'll need to improve this, but that can be a later patch.

With this basic support, this can only boot into kernel directly.
However this is just enough for the petitboot kernel and initradmdisk to
boot from any possible source. Note this requires reasonably recent guest
kernel with:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=df5be5be8735

The immediate benefit is much faster booting time which especially
crucial with fully emulated early CPU bring up environments. Also this
may come handy when/if GRUB-in-the-userspace sees light of the day.

This separates VOF and sPAPR in a hope that VOF bits may be reused by
other POWERPC boards which do not support pSeries.

This is coded in assumption that later on we might be adding support for
booting from QEMU backends (blockdev is the first candidate) without
devices/drivers in between as OF1275 does not require that and
it is quite easy to so.

Signed-off-by: Alexey Kardashevskiy
---

The example command line is:

/home/aik/pbuild/qemu-killslof-localhost-ppc64/qemu-system-ppc64 \
-nodefaults \
-chardev stdio,id=STDIO0,signal=off,mux=on \
-device spapr-vty,id=svty0,reg=0x71000110,chardev=STDIO0 \
-mon id=MON0,chardev=STDIO0,mode=readline \
-nographic \
-vga none \
-enable-kvm \
-m 2G \
-machine
pseries,x-vof=on,cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,cap-ccf-assist=off
\
-kernel pbuild/kernel-le-guest/vmlinux \
-initrd pb/rootfs.cpio.xz \
-drive
id=DRIVE0,if=none,file=./p/qemu-killslof/pc-bios/vof/nvram.bin,format=raw \
-global spapr-nvram.drive=DRIVE0 \
-snapshot \
-smp 8,threads=8 \
-L /home/aik/t/qemu-ppc64-bios/ \
-trace events=qemu_trace_events \
-d guest_errors \
-chardev socket,id=SOCKET0,server,nowait,path=qemu.mon.tmux26 \
-mon chardev=SOCKET0,mode=control

---
Changes:
v14:
* check for truncates in readstr()
* ditched a separate vof_reset()
* spapr->vof is a pointer now, dropped the "on" field
* removed rtas_base from vof and updated comment why we allow setting it
* added myself to maintainers
* updated commit log about blockdev and other possible platforms
* added a note why new hcall is 0x5
* no in place endianness convertion in spapr_h_vof_client
* converted all cpu_physical_memory_read/write to address_space_rw
* git mv hw/ppc/spapr_vof_client.c hw/ppc/spapr_vof.c

v13:
* rebase on latest ppc-for-6.0
* shuffled code around to touch spapr.c less

v12:
* split VOF and SPAPR

v11:
* added g_autofree
* fixed gcc warnings
* fixed few leaks
* added nvram image to make "nvram --print-config" not crash;
Note that contrary to MIN_NVRAM_SIZE (8 * KiB), the actual minimum size
is 16K, or it just does not work (empty output from "nvram")

Re: Some more questions with regards to QEMU clock record and replay

2021-03-01 Thread Pavel Dovgalyuk


On 01.03.2021 20:16, Arnabjyoti Kalita wrote:

Hello all,

I am really thankful for the wonderful answers in my last post linked below-

https://lists.nongnu.org/archive/html/qemu-discuss/2021-02/msg00131.html

In continuation with the last post, I have a few more questions to ask -

My experiment is still, mostly the same. I record clock values in KVM 
mode, and then replay the clock values in TCG mode. However, now I am 
recording and replaying all of the clock values (I was only 
recording/replaying the host clock previously). However, I do not use 
the -icount feature.


- Why are clock values being replayed at checkpoints?


Timers are replayed at checkpoints to be synchronized with vCPU.
Other clock requests (e.g., caused by vCPU instruction) are replayed 
immediately.


- Can we ignore replaying at checkpoints and do a dumb replay as and 
when the clock read actually happens?


I think we can, if we need just clock synchronization.

- Based on the documentation available, I can see that checkpoints are 
necessary for thread synchronization. Does this mean, if I do not replay 
clock values at checkpoints, the guest kernel scheduler might behave 
incorrectly during replay ?


Checkpoints are related to QEMU threads, not guest threads.
Timers are needed for virtual devices, that can generate interrupts, DMA 
requests and so on. Therefore we synchronize them with vCPU to make 
execution deterministic.



Pavel Dovgalyuk

Re: [PATCH V3 05/10] sungem: switch to use qemu_receive_packet() for loopback

2021-03-01 Thread Mark Cave-Ayland


On 02/03/2021 05:54, Jason Wang wrote:


This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Jason Wang 
---
  hw/net/sungem.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/sungem.c b/hw/net/sungem.c
index 33c3722df6..3684a4d733 100644
--- a/hw/net/sungem.c
+++ b/hw/net/sungem.c
@@ -306,7 +306,7 @@ static void sungem_send_packet(SunGEMState *s, const 
uint8_t *buf,
  NetClientState *nc = qemu_get_queue(s->nic);
  
  if (s->macregs[MAC_XIFCFG >> 2] & MAC_XIFCFG_LBCK) {

-nc->info->receive(nc, buf, size);
+qemu_receive_packet(nc, buf, size);
  } else {
  qemu_send_packet(nc, buf, size);
  }


Reviewed-by: Mark Cave-Ayland 


ATB,

Mark.

Re: [PATCH] i386/acpi: restore device paths for pre-5.1 vms

2021-03-01 Thread Thomas Lamprecht

On 01.03.21 20:59, Vitaly Cheptsov wrote:
> After fixing the _UID value for the primary PCI root bridge in
> af1b80ae it was discovered that this change updates Windows
> configuration in an incompatible way causing network configuration
> failure unless DHCP is used. More details provided on the list:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg08484.html
> 
> This change reverts the _UID update from 1 to 0 for q35 and i440fx
> VMs before version 5.2 to maintain the original behaviour when
> upgrading.
> 
> Cc: qemu-sta...@nongnu.org
> Cc: qemu-devel@nongnu.org
> Reported-by: Thomas Lamprecht 
> Suggested-by: Michael S. Tsirkin 
> Signed-off-by: Vitaly Cheptsov 

Thanks for sending this! Works as advertised and can be cleanly cherry-picked
on top of the v5.2.0 tag.

Tested-by: Thomas Lamprecht

Re: [RFC PATCH v2 3/3] hw/block/pflash: use memory_region_init_rom_device_from_file()

On 3/1/21 7:13 PM, Stefan Hajnoczi wrote:
> On Mon, Mar 01, 2021 at 12:53:29PM +0100, Philippe Mathieu-Daudé wrote:
>> If the block drive is read-only we will model a "protected" flash
>> device. We can thus use memory_region_init_rom_device_from_file()
>> which mmap the backing file when creating the MemoryRegion.
>> If the same backing file is used by multiple QEMU instances, this
>> reduces the memory footprint (this is often the case with the
>> CODE flash image from OVMF and AAVMF).
>>
>> Suggested-by: Stefan Hajnoczi 
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>>  hw/block/pflash_cfi01.c | 39 +++
>>  1 file changed, 31 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
>> index a5fa8d8b74a..ec290636298 100644
>> --- a/hw/block/pflash_cfi01.c
>> +++ b/hw/block/pflash_cfi01.c
>> @@ -702,6 +702,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
>> **errp)
>>  int ret;
>>  uint64_t blocks_per_device, sector_len_per_device, device_len;
>>  int num_devices;
>> +bool romd_mr_shared_mapped;
>>  
>>  if (pfl->sector_len == 0) {
>>  error_setg(errp, "attribute \"sector-length\" not specified or 
>> zero.");
>> @@ -743,19 +744,41 @@ static void pflash_cfi01_realize(DeviceState *dev, 
>> Error **errp)
>>  pfl->ro = 0;
>>  }
>>  
>> -memory_region_init_rom_device(
>> ->mem, OBJECT(dev),
>> -_cfi01_ops,
>> -pfl,
>> -pfl->name, total_len, errp);
>> -if (*errp) {
>> -return;
>> +if (pfl->ro && pfl->blk) {
>> +BlockDriverState *bs = blk_bs(pfl->blk);
>> +
>> +/* If "raw" driver used, try to mmap the backing file as RAM_SHARED 
>> */
>> +if (bs->drv == _raw) { /* FIXME check offset=0 ? */
> 
> Bypassing the block layer is tricky because there are a lot of features
> that conflict (you already pointed out the offset= option). Checking
> bdrv_raw is not enough because the underlying protocol driver could be
> GlusterFS, iSCSI, etc.

OK.

> I think the goal here is to avoid changing the command-line/QMP so that
> users don't need to modify their guests. Therefore changing the pflash
> qdev properties is not desirable (we could have added a separate code
> path that bypasses the block layer cleanly).

Yes, this is the limitation.

> This seems like a
> worthwhile optimization that the block layer should support. I suggest
> adding a new API like:
> 
>   /* Returns a filename string if @blk supports read-only mmap */
>   char *blk_get_read_only_mmap_filename(BlockBackend *blk, Error **errp);
> 
> Then block/raw-format.c would forward the call to bs->file and
> block/raw-posix.c would implement it by returning a new filename string
> when bs->read_only is true.

Thanks :) Kevin suggested something similar too.

> 
> FWIW this API isn't perfect because the file could be reopened with QMP
> and the existing mmap would remain in place.

Can you show me a QMP example or point me at the command?
This shouldn't happen with the pflash.

Thanks for reviewing,

Phil.

Re: [PATCH V3 03/10] dp8393x: switch to use qemu_receive_packet() for loopback packet

On 3/2/21 6:54 AM, Jason Wang wrote:
> This patch switches to use qemu_receive_packet() which can detect
> reentrancy and return early.
> 
> Signed-off-by: Jason Wang 
> ---
>  hw/net/dp8393x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé

Re: [PATCH V3 10/10] lan9118: switch to use qemu_receive_packet() for loopback

On 3/2/21 6:55 AM, Jason Wang wrote:
> From: Alexander Bulekov 
> 
> This patch switches to use qemu_receive_packet() which can detect
> reentrancy and return early.
> 
> Signed-off-by: Alexander Bulekov 
> ---
>  hw/net/lan9118.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé

Re: [PATCH V3 09/10] cadence_gem: switch to use qemu_receive_packet() for loopback

On 3/2/21 6:54 AM, Jason Wang wrote:
> From: Alexander Bulekov 
> 
> This patch switches to use qemu_receive_packet() which can detect
> reentrancy and return early.
> 
> Signed-off-by: Alexander Bulekov 
> ---
>  hw/net/cadence_gem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 7a534691f1..6032395388 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -1275,7 +1275,7 @@ static void gem_transmit(CadenceGEMState *s)
>  /* Send the packet somewhere */
>  if (s->phy_loop || (s->regs[GEM_NWCTRL] &
>  GEM_NWCTRL_LOCALLOOP)) {
> -gem_receive(qemu_get_queue(s->nic), s->tx_packet,
> +qemu_receive_packet(qemu_get_queue(s->nic), s->tx_packet,
>  total_bytes);

Indent now off, otherwise:
Reviewed-by: Philippe Mathieu-Daudé 

>  } else {
>  qemu_send_packet(qemu_get_queue(s->nic), s->tx_packet,
>

Re: [PATCH V3 07/10] rtl8139: switch to use qemu_receive_packet() for loopback

On 3/2/21 6:54 AM, Jason Wang wrote:
> From: Alexander Bulekov 
> 
> This patch switches to use qemu_receive_packet() which can detect
> reentrancy and return early.
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1910826
> Signed-off-by: Alexander Bulekov 

Missing your S-o-b?

> ---
>  hw/net/rtl8139.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé

Re: [PATCH V2 7/7] rtl8193: switch to use qemu_receive_packet() for loopback




On 2021/3/2 2:39 下午, P J P wrote:

+-- On Tue, 2 Mar 2021, Jason Wang wrote --+
|  DPRINTF("+++ transmit loopback mode\n");
| -rtl8139_do_receive(qemu_get_queue(s->nic), buf, size, do_interrupt);
| +qemu_receive_packet(qemu_get_queue(s->nic), buf, size);
|
...
|[PATCH V2 7/7] rtl8193: switch to use qemu_receive_packet() for loopback

* Patch 'V2' need not be here.

Thank you.



Right, looks like a stale patch in the directory.

Will not apply this one when mergeing the series.

Thanks



--
  - P J P
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D

[Bug 1917161] Re: Parameter 'type' expects a netdev backend type

2021-03-01 Thread Thomas Huth

Yes, QEMU should come with the libslirp sources. Are you using git? Then
maybe something went wrong with the checkout of the submodule. Is there
something in your "slirp" folder? What do you get when you run "git
submodule" ?

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1917161

Title:
  Parameter 'type' expects a netdev backend type

Status in QEMU:
  Incomplete

Bug description:
  When using QEMU on an M1 Mac with Mac OS 11.1, I see this error
  message when trying to enable networking for a guest:

  Parameter 'type' expects a netdev backend type

  Example command:
  qemu-system-i386 -m 700 -hda  -netdev user,id=n0 -device 
rtl8139,netdev=n0

  What should happen is networking should work when issuing the above
  command. What actually happens is QEMU exits immediately.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1917161/+subscriptions

Re: CI with --enable-debug?

Thomas Huth  writes:

> On 01/03/2021 15.06, Markus Armbruster wrote:
>> Today I debugged why Paolo couldn't reproduce an assertion failure I
>> found in review.  Turns out compiling with optimization masks it for
>> both of us.
>> This made me wonder whether our CI tests with and without
>> optimization.
>> I quick grep finds --enable-debug in .travis.yml, but not in .gitlab*.
>> Is this a gap?
>
> When did you update your local repo the last time? There should be at
> least one --enable-debug in the gitlab CI now, see commit
> ac6d7074c0751f6.

I figure I had switched to an elderly review branch, and forgot to grep
master instead of HEAD, so I missed this change.  Sorry for the noise!

[...]

Re: [PATCH V2 7/7] rtl8193: switch to use qemu_receive_packet() for loopback

2021-03-01 Thread P J P



+-- On Tue, 2 Mar 2021, Jason Wang wrote --+
|  DPRINTF("+++ transmit loopback mode\n");
| -rtl8139_do_receive(qemu_get_queue(s->nic), buf, size, do_interrupt);
| +qemu_receive_packet(qemu_get_queue(s->nic), buf, size);
|  
...
|[PATCH V2 7/7] rtl8193: switch to use qemu_receive_packet() for loopback

* Patch 'V2' need not be here.

Thank you.
--
 - P J P
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D

Re: [PATCH v2 2/2] storage-daemon: include current command line option in the errors

Paolo Bonzini  writes:

> Use the location management facilities that the emulator uses, so that
> the current command line option appears in the error message.
>
> Before:
>
>   $ storage-daemon/qemu-storage-daemon --nbd key..=
>   qemu-storage-daemon: Invalid parameter 'key..'
>
> After:
>
>   $ storage-daemon/qemu-storage-daemon --nbd key..=
>   qemu-storage-daemon: --nbd key..=: Invalid parameter 'key..'
>
> Reviewed-by: Eric Blake 
> Signed-off-by: Paolo Bonzini 

I have a similar patch in an unfinished branch.  You win :)

> ---
>  storage-daemon/qemu-storage-daemon.c | 17 -
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/storage-daemon/qemu-storage-daemon.c 
> b/storage-daemon/qemu-storage-daemon.c
> index 9aa82e7d96..78ddf619d4 100644
> --- a/storage-daemon/qemu-storage-daemon.c
> +++ b/storage-daemon/qemu-storage-daemon.c
> @@ -152,6 +152,20 @@ static void init_qmp_commands(void)
>   qmp_marshal_qmp_capabilities, QCO_ALLOW_PRECONFIG);
>  }
>  
> +static int getopt_set_loc(int argc, char **argv, const char *optstring,
> +  const struct option *longopts)
> +{
> +int c, save_index;
> +
> +optarg = NULL;
> +save_index = optind;
> +c = getopt_long(argc, argv, optstring, longopts, NULL);
> +if (optarg) {
> +loc_set_cmdline(argv, save_index, MAX(1, optind - save_index));
> +}
> +return c;
> +}
> +

I think this function is more widely applicable:

$ git-grep -l getopt_long | xargs grep -l error_report
qemu-img.c
qemu-io.c
qemu-nbd.c
scsi/qemu-pr-helper.c
storage-daemon/qemu-storage-daemon.c

>  static void process_options(int argc, char *argv[])
>  {
>  int c;
> @@ -174,7 +188,7 @@ static void process_options(int argc, char *argv[])
>   * they are given on the command lines. This means that things must be
>   * defined first before they can be referenced in another option.
>   */
> -while ((c = getopt_long(argc, argv, "-hT:V", long_options, NULL)) != -1) 
> {
> +while ((c = getopt_set_loc(argc, argv, "-hT:V", long_options)) != -1) {
>  switch (c) {
>  case '?':
>  exit(EXIT_FAILURE);
> @@ -275,12 +289,13 @@ static void process_options(int argc, char *argv[])
>  break;
>  }
>  case 1:
> -error_report("Unexpected argument: %s", optarg);
> +error_report("Unexpected argument");
>  exit(EXIT_FAILURE);
>  default:
>  g_assert_not_reached();
>  }
>  }
> +loc_set_none();
>  }
>  
>  int main(int argc, char *argv[])

Re: [PATCH v2 1/2] storage-daemon: report unexpected arguments on the fly

Eric Blake  writes:

> On 3/1/21 9:28 AM, Paolo Bonzini wrote:
>> If the first character of optstring is '-', then each nonoption argv
>> element is handled as if it were the argument of an option with character
>> code 1.  This removes the reordering of the argv array, and enables usage
>> of loc_set_cmdline to provide better error messages.
>> 
>> Signed-off-by: Paolo Bonzini 
>> ---
>>  storage-daemon/qemu-storage-daemon.c | 9 -
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> Nice.  The man page for 'getopt_long' is unclear whether setting
> POSIXLY_CORRECT in the environment would break this (that is, setting
> POSIXLY_CORRECT has the same effect as a leading '+'; but you can't have
> both leading '+' and leading '-' and when both are set, it is not clear
> which one wins).  But that's a corner case that I don't think will ever
> bite us in real life.
>
> Reviewed-by: Eric Blake 

I'd consider environment overruling the programmer's express intent a
bug.

GLibc's _getopt_initialize():

  /* Determine how to handle the ordering of options and nonoptions.  */
  if (optstring[0] == '-')
{
  d->__ordering = RETURN_IN_ORDER;
  ++optstring;
}
  else if (optstring[0] == '+')
{
  d->__ordering = REQUIRE_ORDER;
  ++optstring;
}
  else if (posixly_correct || !!getenv ("POSIXLY_CORRECT"))
d->__ordering = REQUIRE_ORDER;
  else
d->__ordering = PERMUTE;

No surprises here.

Re: [PATCH v2 0/5] hw/block/nvme: misc fixes

2021-03-01 Thread Klaus Jensen

On Feb 22 19:47, Klaus Jensen wrote:
> From: Klaus Jensen 
> 
> Small set of misc fixes from Gollu.
> 
> v2 changes
> 
>   * Split off the trace event additions from "[PATCH 1/3] hw/block/nvme:
> nvme_identify fixes" and "[PATCH 2/3] hw/block/nvme: fix potential
> compilation error" into their own commits (Minwoo, Philippe)
>   * Fix a missing check on the zasl_bs param in the
> nvme_identify_ctrl_csi refactor (Minwoo)
> 
> Gollu Appalanaidu (5):
>   hw/block/nvme: remove unnecessary endian conversion
>   hw/block/nvme: add identify trace event
>   hw/block/nvme: fix potential compilation error
>   hw/block/nvme: add trace event for zone read check
>   hw/block/nvme: report non-mdts command size limit for dsm
> 
>  hw/block/nvme.h   |  1 +
>  include/block/nvme.h  | 11 +++
>  hw/block/nvme.c   | 45 ---
>  hw/block/trace-events |  2 ++
>  4 files changed, 43 insertions(+), 16 deletions(-)
> 

Applied to nvme-next!


signature.asc
Description: PGP signature

Re: [RFC v2 19/24] target/arm: move aarch64_sync_32_to_64 (and vv) to cpu code


On 3/1/21 8:49 AM, Claudio Fontana wrote:

and arm_phys_excp_target_el since it is tied up inside the
same #ifdef block.

aarch64_sync_32_to_64 and aarch64_sync_64_to_32 are
mixed in with the tcg helpers, but they shouldn't, as they
are needed for kvm too, in the sysemu case.


Really?  Now that *is* surprising.  Again, please document.  Because I surely 
cannot believe this to be used by kvm.



r~

Re: [RFC v2 06/24] target/arm: split off cpu-sysemu.c


On 3/1/21 8:49 AM, Claudio Fontana wrote:

Signed-off-by: Claudio Fontana
---
  target/arm/internals.h  |   8 ++-
  target/arm/cpu-sysemu.c | 105 
  target/arm/cpu.c|  83 ---
  target/arm/meson.build  |   1 +
  4 files changed, 113 insertions(+), 84 deletions(-)
  create mode 100644 target/arm/cpu-sysemu.c


It'd be nice to rearrange this into tcg/ and kvm/.

I think we could do with some macros like

#ifndef CONFIG_KVM
#define KVM_ERROR  QEMU_ERROR("kvm is disabled")
#endif
#ifndef CONFIG_TCG
#define TCG_ERROR  QEMU_ERROR("tcg is disabled")
#endif

Not sure where to put these, but certainly not arm specific.

Then,

void arm_cpu_tcg_set_irq(void *opaque, int irq, int level)
TCG_ERROR;
void arm_cpu_kvm_set_irq(void *opaque, int irq, int level)
KVM_ERROR;

if (kvm_enabled()) {
qdev_init_gpio_in(DEVICE(cpu), arm_cpu_kvm_set_irq, 4);
} else if (tcg_enabled()) {
qdev_init_gpio_in(DEVICE(cpu), arm_cpu_tcg_set_irq, 4);
} else {
g_assert_not_reached();
}

So arm_cpu_kvm_set_irq can go in kvm/ and needs no ifdef.

I'll let folks interested in xen and hvf figure our what needs doing with the 
above.



r~

Re: [PATCH v3 0/3] vfio: Some fixes and optimizations for VFIO migration

2021-03-01 Thread Shenming Lu

Hi Alex,

Does this series need any further modification? Wish you can pick it up. :-)

On 2021/2/23 10:22, Shenming Lu wrote:
> This patch set includes two fixes and one optimization for VFIO migration
> as blew:
> 
> Patch 1-2:
> - Fix two ordering problems in migration.
> 
> Patch 3:
> - Optimize the enabling process of the MSI-X vectors in migration.
> 
> History:
> 
> v2 -> v3:
> - Nit fixes.
> - Set error in migration stream for migration to fail in Patch 1.
> - Tested Patch 3 with a Windows guest.
> 
> Thanks,
> Shenming
> 
> 
> Shenming Lu (3):
>   vfio: Move the saving of the config space to the right place in VFIO
> migration
>   vfio: Set the priority of the VFIO VM state change handler explicitly
>   vfio: Avoid disabling and enabling vectors repeatedly in VFIO
> migration
> 
>  hw/pci/msix.c |  2 +-
>  hw/vfio/migration.c   | 28 +---
>  hw/vfio/pci.c | 20 +---
>  include/hw/pci/msix.h |  1 +
>  4 files changed, 36 insertions(+), 15 deletions(-)
>

[PATCH V3 10/10] lan9118: switch to use qemu_receive_packet() for loopback

From: Alexander Bulekov 

This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

Signed-off-by: Alexander Bulekov 
---
 hw/net/lan9118.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
index abc796285a..6aff424cbe 100644
--- a/hw/net/lan9118.c
+++ b/hw/net/lan9118.c
@@ -680,7 +680,7 @@ static void do_tx_packet(lan9118_state *s)
 /* FIXME: Honor TX disable, and allow queueing of packets.  */
 if (s->phy_control & 0x4000)  {
 /* This assumes the receive routine doesn't touch the VLANClient.  */
-lan9118_receive(qemu_get_queue(s->nic), s->txp->data, s->txp->len);
+qemu_receive_packet(qemu_get_queue(s->nic), s->txp->data, s->txp->len);
 } else {
 qemu_send_packet(qemu_get_queue(s->nic), s->txp->data, s->txp->len);
 }
-- 
2.24.3 (Apple Git-128)

[PATCH V3 09/10] cadence_gem: switch to use qemu_receive_packet() for loopback

From: Alexander Bulekov 

This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

Signed-off-by: Alexander Bulekov 
---
 hw/net/cadence_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 7a534691f1..6032395388 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1275,7 +1275,7 @@ static void gem_transmit(CadenceGEMState *s)
 /* Send the packet somewhere */
 if (s->phy_loop || (s->regs[GEM_NWCTRL] &
 GEM_NWCTRL_LOCALLOOP)) {
-gem_receive(qemu_get_queue(s->nic), s->tx_packet,
+qemu_receive_packet(qemu_get_queue(s->nic), s->tx_packet,
 total_bytes);
 } else {
 qemu_send_packet(qemu_get_queue(s->nic), s->tx_packet,
-- 
2.24.3 (Apple Git-128)

Re: [PATCH v3 09/16] qapi/expr.py: Consolidate check_if_str calls in check_if

John Snow  writes:

> On 2/25/21 9:23 AM, Markus Armbruster wrote:
>> John Snow  writes:
>> 
>>> This is a small rewrite to address some minor style nits.
>>>
>>> Don't compare against the empty list to check for the empty condition, and
>>> move the normalization forward to unify the check on the now-normalized
>>> structure.
>>>
>>> With the check unified, the local nested function isn't needed anymore
>>> and can be brought down into the normal flow of the function. With the
>>> nesting level changed, shuffle the error strings around a bit to get
>>> them to fit in 79 columns.
>>>
>>> Note: though ifcond is typed as Sequence[str] elsewhere, we *know* that
>>> the parser will produce real, bona-fide lists. It's okay to check
>>> isinstance(ifcond, list) here.
>>>
>>> Signed-off-by: John Snow 
>>> ---
>>>   scripts/qapi/expr.py | 36 
>>>   1 file changed, 16 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
>>> index df6c64950fa..3235a3b809e 100644
>>> --- a/scripts/qapi/expr.py
>>> +++ b/scripts/qapi/expr.py
>>> @@ -128,30 +128,26 @@ def check_flags(expr: Expression, info: 
>>> QAPISourceInfo) -> None:
>>>   
>>>   def check_if(expr: _JSObject, info: QAPISourceInfo, source: str) -> None:
>>>   
>>> -def check_if_str(ifcond: object) -> None:
>>> -if not isinstance(ifcond, str):
>>> -raise QAPISemError(
>>> -info,
>>> -"'if' condition of %s must be a string or a list of 
>>> strings"
>>> -% source)
>>> -if ifcond.strip() == '':
>>> -raise QAPISemError(
>>> -info,
>>> -"'if' condition '%s' of %s makes no sense"
>>> -% (ifcond, source))
>>> -
>>>   ifcond = expr.get('if')
>>>   if ifcond is None:
>>>   return
>>> -if isinstance(ifcond, list):
>>> -if ifcond == []:
>>> +
>>> +# Normalize to a list
>>> +if not isinstance(ifcond, list):
>>> +ifcond = [ifcond]
>>> +expr['if'] = ifcond
>>> +
>>> +if not ifcond:
>>> +raise QAPISemError(info, f"'if' condition [] of {source} is 
>>> useless")
>> 
>> In the old code, the connection between the conditional and the error
>> message was a bit more obvious.
>> 
>
> I will admit to that being true.
>
> Do you think it's still worth the change? I do need to get rid of the 
> comparison against "[]", the rest was just "Ah, while I'm here, ..." and 
> I thought it was nice to get rid of the nested function.
>
> (I think it's still worth it.)
>
>>> +
>>> +for element in ifcond:
>> 
>> @element is rather long.  If you hate @elt, what about @ifc?
>> 
>
> Hate's a strong word, It just wasn't obvious to me at the time. I 
> decided to expand it to what you said it stood for.
>
> I can undo that if you are attached to 'elt', but I don't share the view 
> that 'element' is somehow burdensomely long.

I like my loop control variables *short*.

@elt is short.  It's also inexpressive.  That's why I offered @ifc as an
alternative.  I believe "for ifc in ifcond" reads fine.  The abbreviation
is obvious.

@element isn't short, and just as inexpressive as @elt.  It pushes one
line right to PEP 8's length limit.

>>> +if not isinstance(element, str):
>>> +raise QAPISemError(info, (
>>> +f"'if' condition of {source}"
>>> +" must be a string or a list of strings"))
>>> +if element.strip() == '':
>>>   raise QAPISemError(
>>> -info, "'if' condition [] of %s is useless" % source)
>>> -for elt in ifcond:
>>> -check_if_str(elt)
>>> -else:
>>> -check_if_str(ifcond)
>>> -expr['if'] = [ifcond]
>>> +info, f"'if' condition '{element}' of {source} makes no 
>>> sense")
>>>   
>>>   
>>>   def normalize_members(members: object) -> None:
>> 
>> Perhaps:
>> 
>> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
>> index df6c64950f..e904924599 100644
>> --- a/scripts/qapi/expr.py
>> +++ b/scripts/qapi/expr.py
>> @@ -128,30 +128,26 @@ def check_flags(expr: Expression, info: 
>> QAPISourceInfo) -> None:
>>   
>>   def check_if(expr: _JSObject, info: QAPISourceInfo, source: str) -> None:
>>   
>> -def check_if_str(ifcond: object) -> None:
>> -if not isinstance(ifcond, str):
>> -raise QAPISemError(
>> -info,
>> -"'if' condition of %s must be a string or a list of strings"
>> -% source)
>> -if ifcond.strip() == '':
>> -raise QAPISemError(
>> -info,
>> -"'if' condition '%s' of %s makes no sense"
>> -% (ifcond, source))
>> -
>>   ifcond = expr.get('if')
>>   if ifcond is None:
>>   return
>> +
>>   if isinstance(ifcond, list):
>>   if ifcond == []:
>
> Should be "if not ifcond", though I suppose pylint does not

Re: [PATCH] net: validate that ids are well formed




On 2021/3/1 11:07 下午, Eric Blake wrote:

On 3/1/21 8:56 AM, Paolo Bonzini wrote:

When a network or network device is created from the command line or HMP,
QemuOpts ensures that the id passes the id_wellformed check.  However,
QMP skips this:

$ qemu-system-x86_64 -qmp stdio -S -nic user,id=123/456
qemu-system-x86_64: -nic user,id=123/456: Parameter id expects an identifier
Identifiers consist of letters, digits, -, ., _, starting with a letter.

$ qemu-system-x86_64 -qmp stdio -S
{"execute":"qmp_capabilities"}
{"return": {}}
{"execute":"netdev_add", "arguments": {"type": "user", "id": "123/456"}}
{"return": {}}

After:

$ qemu-system-x86_64 -qmp stdio -S
{"execute":"qmp_capabilities"}
{"return": {}}
{"execute":"netdev_add", "arguments": {"type": "user", "id": "123/456"}}
{"error": {"class": "GenericError", "desc": "Parameter "id" expects an 
identifier"}}

Validity checks should be performed always at the bottom of the call chain,
because QMP skips all the steps above.  Do this for the network subsystem.

Cc: Jason Wang 
Signed-off-by: Paolo Bonzini 
---
  net/net.c | 12 
  1 file changed, 12 insertions(+)

Reviewed-by: Eric Blake 



Queued.

Thanks

[PATCH V3 08/10] pcnet: switch to use qemu_receive_packet() for loopback

From: Alexander Bulekov 

This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

Buglink: https://bugs.launchpad.net/qemu/+bug/1917085
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/net/pcnet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index f3f18d8598..dcd3fc4948 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -1250,7 +1250,7 @@ txagain:
 if (BCR_SWSTYLE(s) == 1)
 add_crc = !GET_FIELD(tmd.status, TMDS, NOFCS);
 s->looptest = add_crc ? PCNET_LOOPTEST_CRC : PCNET_LOOPTEST_NOCRC;
-pcnet_receive(qemu_get_queue(s->nic), s->buffer, s->xmit_pos);
+qemu_receive_packet(qemu_get_queue(s->nic), s->buffer, 
s->xmit_pos);
 s->looptest = 0;
 } else {
 if (s->nic) {
-- 
2.24.3 (Apple Git-128)

[PATCH V3 01/10] net: introduce qemu_receive_packet()

Some NIC supports loopback mode and this is done by calling
nc->info->receive() directly which in fact suppresses the effort of
reentrancy check that is done in qemu_net_queue_send().

Unfortunately we can't use qemu_net_queue_send() here since for
loopback there's no sender as peer, so this patch introduce a
qemu_receive_packet() which is used for implementing loopback mode
for a NIC with this check.

NIC that supports loopback mode will be converted to this helper.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Jason Wang 
---
 include/net/net.h   |  5 +
 include/net/queue.h |  8 
 net/net.c   | 38 +++---
 net/queue.c | 22 ++
 4 files changed, 66 insertions(+), 7 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 919facaad2..4f56cae0fa 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -144,12 +144,17 @@ void *qemu_get_nic_opaque(NetClientState *nc);
 void qemu_del_net_client(NetClientState *nc);
 typedef void (*qemu_nic_foreach)(NICState *nic, void *opaque);
 void qemu_foreach_nic(qemu_nic_foreach func, void *opaque);
+int qemu_can_receive_packet(NetClientState *nc);
 int qemu_can_send_packet(NetClientState *nc);
 ssize_t qemu_sendv_packet(NetClientState *nc, const struct iovec *iov,
   int iovcnt);
 ssize_t qemu_sendv_packet_async(NetClientState *nc, const struct iovec *iov,
 int iovcnt, NetPacketSent *sent_cb);
 ssize_t qemu_send_packet(NetClientState *nc, const uint8_t *buf, int size);
+ssize_t qemu_receive_packet(NetClientState *nc, const uint8_t *buf, int size);
+ssize_t qemu_receive_packet_iov(NetClientState *nc,
+const struct iovec *iov,
+int iovcnt);
 ssize_t qemu_send_packet_raw(NetClientState *nc, const uint8_t *buf, int size);
 ssize_t qemu_send_packet_async(NetClientState *nc, const uint8_t *buf,
int size, NetPacketSent *sent_cb);
diff --git a/include/net/queue.h b/include/net/queue.h
index c0269bb1dc..9f2f289d77 100644
--- a/include/net/queue.h
+++ b/include/net/queue.h
@@ -55,6 +55,14 @@ void qemu_net_queue_append_iov(NetQueue *queue,
 
 void qemu_del_net_queue(NetQueue *queue);
 
+ssize_t qemu_net_queue_receive(NetQueue *queue,
+   const uint8_t *data,
+   size_t size);
+
+ssize_t qemu_net_queue_receive_iov(NetQueue *queue,
+   const struct iovec *iov,
+   int iovcnt);
+
 ssize_t qemu_net_queue_send(NetQueue *queue,
 NetClientState *sender,
 unsigned flags,
diff --git a/net/net.c b/net/net.c
index e1035f21d1..6e470133ad 100644
--- a/net/net.c
+++ b/net/net.c
@@ -528,6 +528,17 @@ int qemu_set_vnet_be(NetClientState *nc, bool is_be)
 #endif
 }
 
+int qemu_can_receive_packet(NetClientState *nc)
+{
+if (nc->receive_disabled) {
+return 0;
+} else if (nc->info->can_receive &&
+   !nc->info->can_receive(nc)) {
+return 0;
+}
+return 1;
+}
+
 int qemu_can_send_packet(NetClientState *sender)
 {
 int vm_running = runstate_is_running();
@@ -540,13 +551,7 @@ int qemu_can_send_packet(NetClientState *sender)
 return 1;
 }
 
-if (sender->peer->receive_disabled) {
-return 0;
-} else if (sender->peer->info->can_receive &&
-   !sender->peer->info->can_receive(sender->peer)) {
-return 0;
-}
-return 1;
+return qemu_can_receive_packet(sender->peer);
 }
 
 static ssize_t filter_receive_iov(NetClientState *nc,
@@ -679,6 +684,25 @@ ssize_t qemu_send_packet(NetClientState *nc, const uint8_t 
*buf, int size)
 return qemu_send_packet_async(nc, buf, size, NULL);
 }
 
+ssize_t qemu_receive_packet(NetClientState *nc, const uint8_t *buf, int size)
+{
+if (!qemu_can_receive_packet(nc)) {
+return 0;
+}
+
+return qemu_net_queue_receive(nc->incoming_queue, buf, size);
+}
+
+ssize_t qemu_receive_packet_iov(NetClientState *nc, const struct iovec *iov,
+int iovcnt)
+{
+if (!qemu_can_receive_packet(nc)) {
+return 0;
+}
+
+return qemu_net_queue_receive_iov(nc->incoming_queue, iov, iovcnt);
+}
+
 ssize_t qemu_send_packet_raw(NetClientState *nc, const uint8_t *buf, int size)
 {
 return qemu_send_packet_async_with_flags(nc, QEMU_NET_PACKET_FLAG_RAW,
diff --git a/net/queue.c b/net/queue.c
index 19e32c80fd..c872d51df8 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -182,6 +182,28 @@ static ssize_t qemu_net_queue_deliver_iov(NetQueue *queue,
 return ret;
 }
 
+ssize_t qemu_net_queue_receive(NetQueue *queue,
+   const uint8_t *data,
+   size_t size)
+{
+if (queue->delivering) {
+return 0;
+}
+
+return qemu_net_queue_deliver(queue, NULL, 0,

[PATCH V3 07/10] rtl8139: switch to use qemu_receive_packet() for loopback

From: Alexander Bulekov 

This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

Buglink: https://bugs.launchpad.net/qemu/+bug/1910826
Signed-off-by: Alexander Bulekov 
---
 hw/net/rtl8139.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 4675ac878e..90b4fc63ce 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -1795,7 +1795,7 @@ static void rtl8139_transfer_frame(RTL8139State *s, 
uint8_t *buf, int size,
 }
 
 DPRINTF("+++ transmit loopback mode\n");
-rtl8139_do_receive(qemu_get_queue(s->nic), buf, size, do_interrupt);
+qemu_receive_packet(qemu_get_queue(s->nic), buf, size);
 
 if (iov) {
 g_free(buf2);
-- 
2.24.3 (Apple Git-128)

[PATCH V2 7/7] rtl8193: switch to use qemu_receive_packet() for loopback

From: Alexander Bulekov 

This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

Buglink: https://bugs.launchpad.net/qemu/+bug/1910826
Signed-off-by: Alexander Bulekov 
---
 hw/net/rtl8139.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 4675ac878e..90b4fc63ce 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -1795,7 +1795,7 @@ static void rtl8139_transfer_frame(RTL8139State *s, 
uint8_t *buf, int size,
 }
 
 DPRINTF("+++ transmit loopback mode\n");
-rtl8139_do_receive(qemu_get_queue(s->nic), buf, size, do_interrupt);
+qemu_receive_packet(qemu_get_queue(s->nic), buf, size);
 
 if (iov) {
 g_free(buf2);
-- 
2.24.3 (Apple Git-128)

[PATCH V3 04/10] msf2-mac: switch to use qemu_receive_packet() for loopback

This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Jason Wang 
---
 hw/net/msf2-emac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/msf2-emac.c b/hw/net/msf2-emac.c
index 32ba9e8412..3e6206044f 100644
--- a/hw/net/msf2-emac.c
+++ b/hw/net/msf2-emac.c
@@ -158,7 +158,7 @@ static void msf2_dma_tx(MSF2EmacState *s)
  * R_CFG1 bit 0 is set.
  */
 if (s->regs[R_CFG1] & R_CFG1_LB_EN_MASK) {
-nc->info->receive(nc, buf, size);
+qemu_receive_packet(nc, buf, size);
 } else {
 qemu_send_packet(nc, buf, size);
 }
-- 
2.24.3 (Apple Git-128)

[PATCH V3 02/10] e1000: switch to use qemu_receive_packet() for loopback

This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Jason Wang 
---
 hw/net/e1000.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index d8da2f6528..39141d2764 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -546,7 +546,7 @@ e1000_send_packet(E1000State *s, const uint8_t *buf, int 
size)
 
 NetClientState *nc = qemu_get_queue(s->nic);
 if (s->phy_reg[PHY_CTRL] & MII_CR_LOOPBACK) {
-nc->info->receive(nc, buf, size);
+qemu_receive_packet(nc, buf, size);
 } else {
 qemu_send_packet(nc, buf, size);
 }
-- 
2.24.3 (Apple Git-128)

[PATCH V3 06/10] tx_pkt: switch to use qemu_receive_packet_iov() for loopback

This patch switches to use qemu_receive_receive_iov() which can detect
reentrancy and return early.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Jason Wang 
---
 hw/net/net_tx_pkt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
index da262edc3e..1f9aa59eca 100644
--- a/hw/net/net_tx_pkt.c
+++ b/hw/net/net_tx_pkt.c
@@ -553,7 +553,7 @@ static inline void net_tx_pkt_sendv(struct NetTxPkt *pkt,
 NetClientState *nc, const struct iovec *iov, int iov_cnt)
 {
 if (pkt->is_loopback) {
-nc->info->receive_iov(nc, iov, iov_cnt);
+qemu_receive_packet_iov(nc, iov, iov_cnt);
 } else {
 qemu_sendv_packet(nc, iov, iov_cnt);
 }
-- 
2.24.3 (Apple Git-128)

[PATCH V3 03/10] dp8393x: switch to use qemu_receive_packet() for loopback packet

This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

Signed-off-by: Jason Wang 
---
 hw/net/dp8393x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 205c0decc5..533a8304d0 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -506,7 +506,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
 s->regs[SONIC_TCR] |= SONIC_TCR_CRSL;
 if (nc->info->can_receive(nc)) {
 s->loopback_packet = 1;
-nc->info->receive(nc, s->tx_buffer, tx_len);
+qemu_receive_packet(nc, s->tx_buffer, tx_len);
 }
 } else {
 /* Transmit packet */
-- 
2.24.3 (Apple Git-128)

[PATCH V3 05/10] sungem: switch to use qemu_receive_packet() for loopback

This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Jason Wang 
---
 hw/net/sungem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/sungem.c b/hw/net/sungem.c
index 33c3722df6..3684a4d733 100644
--- a/hw/net/sungem.c
+++ b/hw/net/sungem.c
@@ -306,7 +306,7 @@ static void sungem_send_packet(SunGEMState *s, const 
uint8_t *buf,
 NetClientState *nc = qemu_get_queue(s->nic);
 
 if (s->macregs[MAC_XIFCFG >> 2] & MAC_XIFCFG_LBCK) {
-nc->info->receive(nc, buf, size);
+qemu_receive_packet(nc, buf, size);
 } else {
 qemu_send_packet(nc, buf, size);
 }
-- 
2.24.3 (Apple Git-128)

[PATCH V3 00/10] Detect reentrant RX casued by loopback

Hi All:

Followed by commit 22dc8663d9 ("net: forbid the reentrant RX"), we
still need to fix the issues casued by loopback mode where the NIC
usually it via calling nc->info->receive() directly.

The fix is to introduce new network helper and check the
queue->delivering.

Thanks

Changes since V2:
- add more fixes from Alexander

Changes since V1:

- Fix dp8393x compiling
- Add rtl8139 fix
- Tweak the commit log
- Silent patchew warning

Alexander Bulekov (4):
  rtl8139: switch to use qemu_receive_packet() for loopback
  pcnet: switch to use qemu_receive_packet() for loopback
  cadence_gem: switch to use qemu_receive_packet() for loopback
  lan9118: switch to use qemu_receive_packet() for loopback

Jason Wang (6):
  net: introduce qemu_receive_packet()
  e1000: switch to use qemu_receive_packet() for loopback
  dp8393x: switch to use qemu_receive_packet() for loopback packet
  msf2-mac: switch to use qemu_receive_packet() for loopback
  sungem: switch to use qemu_receive_packet() for loopback
  tx_pkt: switch to use qemu_receive_packet_iov() for loopback

 hw/net/cadence_gem.c |  2 +-
 hw/net/dp8393x.c |  2 +-
 hw/net/e1000.c   |  2 +-
 hw/net/lan9118.c |  2 +-
 hw/net/msf2-emac.c   |  2 +-
 hw/net/net_tx_pkt.c  |  2 +-
 hw/net/pcnet.c   |  2 +-
 hw/net/rtl8139.c |  2 +-
 hw/net/sungem.c  |  2 +-
 include/net/net.h|  5 +
 include/net/queue.h  |  8 
 net/net.c| 38 +++---
 net/queue.c  | 22 ++
 13 files changed, 75 insertions(+), 16 deletions(-)

-- 
2.24.3 (Apple Git-128)

Re: [RFC v2 05/24] target/arm: wrap arm_cpu_exec_interrupt in CONFIG_TCG


On 3/1/21 8:49 AM, Claudio Fontana wrote:

+#ifdef CONFIG_TCG
  bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
  {
  CPUClass *cc = CPU_GET_CLASS(cs);
@@ -607,6 +608,7 @@ bool arm_cpu_exec_interrupt(CPUState *cs, int 
interrupt_request)
  cc->tcg_ops->do_interrupt(cs);
  return true;
  }
+#endif /* CONFIG_TCG */


Drop this and just wait for the move to tcg/tcg-cpu.c.


r~

Re: [RFC v2 18/24] target/arm: move arm_cpu_list to common_cpu


On 3/1/21 8:49 AM, Claudio Fontana wrote:

Signed-off-by: Claudio Fontana
---
  target/arm/cpu-common.c | 42 +
  target/arm/tcg/helper.c | 41 
  2 files changed, 42 insertions(+), 41 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v3 08/16] qapi/expr.py: add type hint annotations

John Snow  writes:

> On 2/25/21 8:56 AM, Markus Armbruster wrote:
>> John Snow  writes:
>> 
>>> Annotations do not change runtime behavior.
>>> This commit *only* adds annotations.
>>>
>>> Signed-off-by: John Snow 
>>> Reviewed-by: Eduardo Habkost 
>>> Reviewed-by: Cleber Rosa 
>>> ---
>>>   scripts/qapi/expr.py  | 71 ---
>>>   scripts/qapi/mypy.ini |  5 ---
>>>   2 files changed, 46 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
>>> index f45d6be1f4c..df6c64950fa 100644
>>> --- a/scripts/qapi/expr.py
>>> +++ b/scripts/qapi/expr.py
>>> @@ -15,7 +15,14 @@
>>>   # See the COPYING file in the top-level directory.
>>>   
>>>   import re
>>> -from typing import MutableMapping, Optional, cast
>>> +from typing import (
>>> +Iterable,
>>> +List,
>>> +MutableMapping,
>>> +Optional,
>>> +Union,
>>> +cast,
>>> +)
>>>   
>>>   from .common import c_name
>>>   from .error import QAPISemError
>>> @@ -23,9 +30,10 @@
>>>   from .source import QAPISourceInfo
>>>   
>>>   
>>> -# Expressions in their raw form are JSON-like structures with arbitrary 
>>> forms.
>>> -# Minimally, their top-level form must be a mapping of strings to values.
>>> -Expression = MutableMapping[str, object]
>>> +# Arbitrary form for a JSON-like object.
>>> +_JSObject = MutableMapping[str, object]
>>> +# Expressions in their raw form are (just) JSON-like objects.
>>> +Expression = _JSObject
>> 
>> We solved a similar, slightly more involved typing problem in
>> introspect.py.
>> 
>> Whereas expr.py uses Python dict, list, and scalars to represent the
>> output of a JSON parser, introspect.py uses them to represent the input
>> of a quasi-JSON formatter ("quasi-JSON" because it spits out a C
>> initializer for a C representation of JSON, but that's detail).
>> 
>> introspect.py additionally supports comments and #if conditionals.
>> 
>> This is the solution we're using in introspect.py.  The Annotated[] part
>> is for comments and conditionals; ignore that.
>> 
>># This module constructs a tree data structure that is used to
>># generate the introspection information for QEMU. It is shaped
>># like a JSON value.
>>#
>># A complexity over JSON is that our values may or may not be annotated.
>>#
>># Un-annotated values may be:
>># Scalar: str, bool, None.
>># Non-scalar: List, Dict
>># _value = Union[str, bool, None, Dict[str, JSONValue], List[JSONValue]]
>>#
>># With optional annotations, the type of all values is:
>># JSONValue = Union[_Value, Annotated[_Value]]
>>#
>># Sadly, mypy does not support recursive types; so the _Stub alias is 
>> used to
>># mark the imprecision in the type model where we'd otherwise use 
>> JSONValue.
>>_Stub = Any
>>_Scalar = Union[str, bool, None]
>>_NonScalar = Union[Dict[str, _Stub], List[_Stub]]
>>_Value = Union[_Scalar, _NonScalar]
>>JSONValue = Union[_Value, 'Annotated[_Value]']
>> 
>> introspect.py then adds some more type aliases to convey meaning:
>> 
>># These types are based on structures defined in QEMU's schema, so we
>># lack precise types for them here. Python 3.6 does not offer
>># TypedDict constructs, so they are broadly typed here as simple
>># Python Dicts.
>>SchemaInfo = Dict[str, object]
>>SchemaInfoObject = Dict[str, object]
>>SchemaInfoObjectVariant = Dict[str, object]
>>SchemaInfoObjectMember = Dict[str, object]
>>SchemaInfoCommand = Dict[str, object]
>> 
>> I'm not asking you to factor out common typing.
>> 
>> I'm not even asking you to rework expr.py to maximize similarity.
>> 
>> I am asking you to consider stealing applicable parts from
>> introspect.py's comments.
>> 
>> _JSObject seems to serve the same purpose as JSONValue.  Correct?
>> 
>> Expression seems to serve a comparable purpose as SchemaInfo.  Correct?
>> 
>> [...]
>> 
>
> Similar, indeed.
>
> Without annotations:
>
> _Stub = Any
> _Scalar = Union[str, bool, None]
> _NonScalar = Union[Dict[str, _Stub], List[_Stub]]
> _Value = Union[_Scalar, _NonScalar]
> JSONValue = _Value
>
> (Or skip the intermediate _Value name. No matter.)
>
> Though expr.py has no use of anything except the Object form itself, 
> because it is inherently a validator and it doesn't actually really 
> require any specific type, necessarily.
>
> So I only really needed the object form, which we never named in 
> introspect.py. We actually avoided naming it.
>
> All I really need is, I think:
>
> _JSONObject = Dict[str, object]
>
> with a comment explaining that object can be any arbitrary JSONValue 
> (within limit for what parser.py is capable of producing), and that the 
> exact form of such will be evaluated by the various check_definition() 
> functions.
>
> Is that suitable, or do you have something else in mind?

Sounds good.

Re: [RFC PATCH] docs/system: add a gentle prompt for the complexity to come

2021-03-01 Thread Thomas Huth


On 01/03/2021 12.27, Alex Bennée wrote:

We all know the QEMU command line can become a fiendishly complex
beast. Lets gently prepare our user for the horrors to come by
referencing where other example command lines can be found in the
manual.

Signed-off-by: Alex Bennée 
---
  docs/system/quickstart.rst | 8 
  docs/system/targets.rst| 2 ++
  2 files changed, 10 insertions(+)

diff --git a/docs/system/quickstart.rst b/docs/system/quickstart.rst
index 3a3acab5e7..3498c5a29f 100644
--- a/docs/system/quickstart.rst
+++ b/docs/system/quickstart.rst
@@ -11,3 +11,11 @@ Download and uncompress a PC hard disk image with Linux 
installed (e.g.
 |qemu_system| linux.img
  
  Linux should boot and give you a prompt.

+
+Users should be aware the above example elides a lot of the complexity
+of setting up a VM with x86_64 specific defaults and an assumption the


s/an assumption/assumes/ ?
(I'm not a native speaker, but to me it sounds like a verb is missing here)


+first non switch argument is a PC compatible disk image with a boot
+sector. For non-x86 system where we emulate a broad range of machine
+types the command lines are generally more explicit in defining the
+machine and boot behaviour. You will find more example command lines
+in the :ref:`system-targets-ref` section of the manual.
diff --git a/docs/system/targets.rst b/docs/system/targets.rst
index 560783644d..145cc64551 100644
--- a/docs/system/targets.rst
+++ b/docs/system/targets.rst
@@ -1,3 +1,5 @@
+.. _system-targets-ref:
+
  QEMU System Emulator Targets
  


With the first sentence fixed:
Reviewed-by: Thomas Huth

Re: [PATCH v3 03/16] qapi/expr.py: constrain incoming expression types

John Snow  writes:

> On 2/25/21 6:56 AM, Markus Armbruster wrote:
>> John Snow  writes:
>> 
>>> On 2/24/21 5:01 AM, Markus Armbruster wrote:
 John Snow  writes:

> mypy does not know the types of values stored in Dicts that masquerade
> as objects. Help the type checker out by constraining the type.
>
> Signed-off-by: John Snow 
> Reviewed-by: Eduardo Habkost 
> Reviewed-by: Cleber Rosa 
> ---
>scripts/qapi/expr.py | 25 ++---
>1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index 5694c501fa3..783282b53ce 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -15,9 +15,17 @@
># See the COPYING file in the top-level directory.
>
>import re
> +from typing import MutableMapping, Optional
>
>from .common import c_name
>from .error import QAPISemError
> +from .parser import QAPIDoc
> +from .source import QAPISourceInfo
> +
> +
> +# Expressions in their raw form are JSON-like structures with arbitrary 
> forms.
> +# Minimally, their top-level form must be a mapping of strings to values.
> +Expression = MutableMapping[str, object]

 MutableMapping, fancy.  It's only ever dict.  Why abstract from that?
>> 
>> OrderedDict, actually.  MutableMapping is misleading, because it doesn't
>> specify "orderedness".
>> 
>
> Yeah, I am realizing that Dict helps imply that constraint on 3.6+ but 
> that MutableMapping doesn't.
>
> I am worried about how hard it's gonna hurt when I remember why I wanted 
> MutableMapping.
>
>  >:|
>
> For now, I'll go back to Dict.
>
>>> I don't know! I referenced this in the cover letter. I cannot remember
>>> the reason anymore. It had R-Bs on it so I left it alone.
>>>
>>> There are some differences, but I no longer remember why I thought they
>>> applied. Maybe some of my more exploratory work wanted it. Dunno.
>> 
>> Happens.  It's a long patch queue you're trying to flush.
>> 
 The use of object is again owed to mypy's inability to do recursive
 types.  What we really have here is something like

  Expression = Union[bool, str, dict[str, Expression], list[Expression]]

 with the root further constrained to the Union's dict branch.  Spell
 that out in a bit more detail, like you did in introspect.py?

>>>
>>> Terminology problem?
>>>
>>> I am using "Expression" to mean very specifically a top-level object as
>>> returned from parser.py, which *must* be an Object, so it *must* be a
>>> mapping of str => yaddayadda.
>> 
>> Aha!
>> 
>> We'll talk some more about naming of type aliases in review of PATCH 08.
>> 
>>> The type as I intended it is Expression = Dict[str, yaddayadda]
>>>
>>> where yaddayadda is
>>> Union[int, str, bool, List[yaddayadda], Dict[str, yaddayadda]]
>> 
>> Yes.
>> 
>> As qapi-code-gen.txt explains, we have two layers of syntax:
>> 
>> * The bottom layer is (heavily bastardized) JSON.  qapi-code-gen.txt
>>specifies it by listing the differences to RFC 8259.  parser.py parses
>>it into abstract syntax trees.
>> 
>
> Aside: A new realization about a deviation from JSON: objects are 
> inherently unordered collections.

For a value of "new" :)

In JSON *syntax* (the thing defined by its grammar), order matters.
It doesn't in *semantics*.  Except when it does:

JSON parsing libraries have been observed to differ as to whether or
not they make the ordering of object members visible to calling
software.  Implementations whose behavior does not depend on member
ordering will be interoperable in the sense that they will not be
affected by these differences.

This is RFC 8259 section 4, Objects.

qapi-code-gen.txt spells it out for the QAPI schema language.  Section
"Schema syntax":

The order of members within JSON objects does not matter unless
explicitly noted.

Later sections note explicitly.

>> * The upper layer recognizes the abstract syntax trees that are valid as
>>QAPI schema.  qapi-code-gen.txt specifies it with a context-free
>>grammar.  expr.py checks the ASTs against that grammar.  It also
>>expands shorthand forms into longhand.
>> 
>> Detail not documented in qapi-code-gen.txt: parser.py rejects non-object
>> at the top-level, so expr.py doesn't have to.
>> 
>
> Yep.
>
>>> expr.py is what validates the yaddayadda, so there's no point in trying
>>> to type it further, I think.
>> 
>> If mypy could do recursive types, typing it further would be a
>> no-brainer: just state what is.
>> 
>> Since it can't, we need to stop typing / start cheating at some point.
>> Where exactly is not obvious.  Your idea is at least as good as mine.
>> 
>>> Probably worth a better comment.
>> 
>> Yes :)
>> 
>
> I'll look at Patch 8 and then revisit, but I will attempt to make a 
> better comment. I think there are bits of part 5 that makes

Re: CI with --enable-debug?

2021-03-01 Thread Thomas Huth


On 01/03/2021 15.06, Markus Armbruster wrote:

Today I debugged why Paolo couldn't reproduce an assertion failure I
found in review.  Turns out compiling with optimization masks it for
both of us.

This made me wonder whether our CI tests with and without optimization.
I quick grep finds --enable-debug in .travis.yml, but not in .gitlab*.
Is this a gap?


When did you update your local repo the last time? There should be at least 
one --enable-debug in the gitlab CI now, see commit ac6d7074c0751f6.


If that didn't catch your problem, it's bad luck. With the slow shared 
runners, we currently cannot test each and every combination. But if you've 
got an idea where to add another switch, feel free to send patches.


 Thomas

Re: [PATCH] docs: show how to spawn qemu-storage-daemon with fd passing

Daniel P. Berrangé  writes:

> On Mon, Mar 01, 2021 at 09:49:21AM -0600, Eric Blake wrote:
>> On 3/1/21 9:41 AM, Daniel P. Berrangé wrote:
>> > On Mon, Mar 01, 2021 at 03:31:59PM +, Stefan Hajnoczi wrote:
>> >> The QMP monitor, NBD server, and vhost-user-blk export all support file
>> >> descriptor passing. This is a useful technique because it allows the
>> >> parent process to spawn and wait for qemu-storage-daemon without busy
>> >> waiting, which may delay startup due to arbitrary sleep() calls.
>> >>
>> >> This Python example is inspired by the test case written for libnbd by
>> >> Richard W.M. Jones :
>> >> https://gitlab.com/nbdkit/libnbd/-/commit/89113f484effb0e6c322314ba75c1cbe07a04543
>> >>
>> >> Thanks to Daniel P. Berrangé  for suggestions on
>> >> how to get this working. Now let's document it!
>> >>
>> 
>> >> +  sock_path = '/tmp/qmp-{}.sock'.format(os.getpid())
>> > 
>> > Example code inevitably gets cut+paste into real world apps, and this
>> > example is a tmpfile CVE flaw. At least put it in $CWD instead.
>> 
>> Except $CWD may be too long for a sock file name to be created.
>> Creating the sock in a securely-created subdirectory of /tmp is more
>> reliable.
>
> $XDG_RUNTIME_DIR then, which is /run/user/$UID, so safely per user on all
> modern OS.

Reach under your pillow and check the standard library:

import tempfile

with tempfile.TemporaryDirectory() as tmpdirname:
print('created temporary directory', tmpdirname)

https://docs.python.org/3.6/library/tempfile.html#tempfile.TemporaryDirectory

Re: [RFC v2 16/24] target/arm: move sve_zcr_len_for_el to common_cpu


On 3/1/21 8:49 AM, Claudio Fontana wrote:

it is required by arch-dump.c and cpu.c, so apparently
we need this for KVM too?

Signed-off-by: Claudio Fontana
---
  target/arm/cpu-common.c | 33 +
  target/arm/tcg/helper.c | 33 -
  2 files changed, 33 insertions(+), 33 deletions(-)


Same about the choice of file as with cpsr in patch 12.  This is 64-bit 
specific at least.



r~

Re: [RFC v2 15/24] target/arm: move arm_mmu_idx* to get-phys-addr


On 3/1/21 8:49 AM, Claudio Fontana wrote:

Signed-off-by: Claudio Fontana
---
  target/arm/get-phys-addr.c | 95 ++
  target/arm/tcg/helper.c| 95 --
  2 files changed, 95 insertions(+), 95 deletions(-)


So... get-phys-addr.c is now gaining stuff not related to get_phys_addr? 
What's your end goal?


Is there a better a better name, like mmu_helper.c?  Should these in fact go 
into a second file, leaving get-phys-addr to itself?



r~

Re: [RFC v2 14/24] target/arm: split vfp state setting from tcg helpers


On 3/1/21 8:49 AM, Claudio Fontana wrote:

vfp_get_fpsr and vfp_set_fpsr are needed also for KVM.

Signed-off-by: Claudio Fontana
---
  target/arm/cpu-vfp.h  |  29 +
  target/arm/cpu-vfp.c  |  92 +++
  target/arm/kvm/helper-stubs.c |  10 ++
  target/arm/tcg/cpu-vfp.c  | 146 +++
  target/arm/tcg/vfp_helper.c   | 210 +-
  target/arm/meson.build|   1 +
  target/arm/tcg/meson.build|   1 +
  7 files changed, 281 insertions(+), 208 deletions(-)
  create mode 100644 target/arm/cpu-vfp.h
  create mode 100644 target/arm/cpu-vfp.c
  create mode 100644 target/arm/tcg/cpu-vfp.c


kvm/helper-stubs.c should be for all !tcg.


r~

Re: [RFC v2 13/24] target/arm: add temporary stub for arm_rebuild_hflags


On 3/1/21 8:49 AM, Claudio Fontana wrote:

+#ifdef CONFIG_TCG
  /* CP15 update requires rebuilding hflags */
  arm_rebuild_hflags(_cpu->env);
+#endif /* CONFIG_TCG */

...

+/* XXX this is used all over in hw/arm, needs Philippe's work to remove */
+void arm_rebuild_hflags(CPUARMState *env)
+{
+g_assert_not_reached();
+}



If you have the stub, you can use tcg_enabled(), not an ifdef.

r~

Re: [RFC v2 12/24] target/arm: move cpsr_read, cpsr_write to cpu_common


On 3/1/21 8:49 AM, Claudio Fontana wrote:

we need as a result to move switch_mode too,
so we put an implementation into cpu_user and cpu_sysemu.

Signed-off-by: Claudio Fontana
---
  target/arm/cpu.h|   2 +
  target/arm/cpu-common.c | 182 +
  target/arm/cpu-sysemu.c |  29 ++
  target/arm/cpu-user.c   |  24 +
  target/arm/tcg/helper.c | 221 
  target/arm/meson.build  |   3 +
  6 files changed, 240 insertions(+), 221 deletions(-)
  create mode 100644 target/arm/cpu-user.c


Some commentary about what cpu-common.c is meant to be would be helpful.  So 
far it looks like helper.c under a different name.


r~

Re: [PATCH v4 0/6] Pegasos2 emulation

On Thu, Feb 25, 2021 at 08:47:36PM +0100, BALATON Zoltan wrote:
> Hello,
> 
> This is adding a new PPC board called pegasos2. More info on it can be
> found at:
> 
> https://osdn.net/projects/qmiga/wiki/SubprojectPegasos2
> 
> Currently it needs a firmware ROM image that I cannot include due to
> original copyright holder (bPlan) did not release it under a free
> licence but I have plans to write a replacement in the future. With
> the original board firmware it can boot MorphOS now as:
> 
> qemu-system-ppc -M pegasos2 -cdrom morphos.iso -device ati-vga,romfile="" 
> -serial stdio
> 
> then enter "boot cd boot.img" at the firmware "ok" prompt as described
> in the MorphOS.readme. To boot Linux use same command line with e.g.
> -cdrom debian-8.11.0-powerpc-netinst.iso then enter
> "boot cd install/pegasos"
> 
> The last patch adds the actual board code after previous patches
> adding VT8231 and MV64361 system controller chip emulation. The
> mv643xx.h header file is taken from Linux and produces a bunch of
> checkpatch warnings due to different formatting rules it follows, I'm
> not sure we want to adopt it and change formatting or keep it as it is.
> 
> Regards,
> BALATON Zoltan

The ppc specific parts (patches 5 & 6) look generally sane to me.  I
haven't looked in great depth, since the threshold for entirely new
devices is pretty low.

I'd be happy to queue this in the ppc tree, but I'll need acks from
the relevant maintainers for the changes to the existing VIA bridge
devices.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH v4 0/6] Pegasos2 emulation

On Thu, Feb 25, 2021 at 12:10:55PM -0800, no-re...@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/cover.1614282456.git.bala...@eik.bme.hu/
> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Type: series
> Message-id: cover.1614282456.git.bala...@eik.bme.hu
> Subject: [PATCH v4 0/6] Pegasos2 emulation
> 
> === 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
>7ef8134..51db2d7  master -> master
>  - [tag update]  patchew/20210225181507.3624509-1-f4...@amsat.org -> 
> patchew/20210225181507.3624509-1-f4...@amsat.org
>  * [new tag] patchew/cover.1614282456.git.bala...@eik.bme.hu -> 
> patchew/cover.1614282456.git.bala...@eik.bme.hu
> Switched to a new branch 'test'
> 42b2a15 hw/ppc: Add emulation of Genesi/bPlan Pegasos II
> 2b43495 hw/pci-host: Add emulation of Marvell MV64361 PPC system controller
> 0ba0ef2 vt82c686: Add emulation of VT8231 south bridge
> 4d547c7 vt82c686: Add VT8231_SUPERIO based on VIA_SUPERIO
> af493ab vt82c686: QOM-ify superio related functionality
> 9041915 vt82c686: Implement control of serial port io ranges via config regs
> 
> === OUTPUT BEGIN ===
> 1/6 Checking commit 904191528152 (vt82c686: Implement control of serial port 
> io ranges via config regs)
> 2/6 Checking commit af493ab3ac51 (vt82c686: QOM-ify superio related 
> functionality)
> 3/6 Checking commit 4d547c7dc19d (vt82c686: Add VT8231_SUPERIO based on 
> VIA_SUPERIO)
> 4/6 Checking commit 0ba0ef255e0d (vt82c686: Add emulation of VT8231 south 
> bridge)
> 5/6 Checking commit 2b43495ccd8f (hw/pci-host: Add emulation of Marvell 
> MV64361 PPC system controller)
> Use of uninitialized value $acpi_testexpected in string eq at 
> ./scripts/checkpatch.pl line 1529.
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #51: 
> new file mode 100644
> 
> WARNING: line over 80 characters
> #307: FILE: hw/pci-host/mv64361.c:252:
> +trace_mv64361_region_enable(!(val & mask) ? "enable" : 
> "disable", i);

You will need to fix these style errors before merge.

> ERROR: code indent should never use tabs
> #1032: FILE: hw/pci-host/mv643xx.h:5:
> + * ^IAuthor: Matthew Dharm $
> 
> WARNING: architecture specific defines should be avoided
> #1040: FILE: hw/pci-host/mv643xx.h:13:
> +#ifndef __ASM_MV643XX_H
> 
> WARNING: Block comments use a leading /* on a separate line
> #1102: FILE: hw/pci-host/mv643xx.h:75:
> +/* Enables the CS , DEV_CS , PCI 0 and PCI 1
> 
> WARNING: Block comments use * on subsequent lines
> #1103: FILE: hw/pci-host/mv643xx.h:76:
> +/* Enables the CS , DEV_CS , PCI 0 and PCI 1
> +   windows above */
> 
> WARNING: Block comments use a trailing */ on a separate line
> #1103: FILE: hw/pci-host/mv643xx.h:76:
> +   windows above */
> 
> ERROR: code indent should never use tabs
> #1206: FILE: hw/pci-host/mv643xx.h:179:
> +/*  CPU Interface Debug Registers ^I*/$
> 
> ERROR: code indent should never use tabs
> #1283: FILE: hw/pci-host/mv643xx.h:256:
> +/* Device Parameters^I^I^I*/$
> 
> ERROR: code indent should never use tabs
> #1286: FILE: hw/pci-host/mv643xx.h:259:
> +#define MV64340_DEVICE_BANK0_PARAMETERS^I^I^I^I0x45c$
> 
> ERROR: code indent should never use tabs
> #1287: FILE: hw/pci-host/mv643xx.h:260:
> +#define MV64340_DEVICE_BANK1_PARAMETERS^I^I^I^I0x460$
> 
> ERROR: code indent should never use tabs
> #1288: FILE: hw/pci-host/mv643xx.h:261:
> +#define MV64340_DEVICE_BANK2_PARAMETERS^I^I^I^I0x464$
> 
> ERROR: code indent should never use tabs
> #1289: FILE: hw/pci-host/mv643xx.h:262:
> +#define MV64340_DEVICE_BANK3_PARAMETERS^I^I^I^I0x468$
> 
> ERROR: code indent should never use tabs
> #1290: FILE: hw/pci-host/mv643xx.h:263:
> +#define MV64340_DEVICE_BOOT_BANK_PARAMETERS^I^I^I0x46c$
> 
> ERROR: code indent should never use tabs
> #1297: FILE: hw/pci-host/mv643xx.h:270:
> +/* Device interrupt registers^I^I*/$
> 
> ERROR: code indent should never use tabs
> #1300: FILE: hw/pci-host/mv643xx.h:273:
> +#define MV64340_DEVICE_INTERRUPT_CAUSE^I^I^I^I0x4d0$
> 
> ERROR: code indent should never use tabs
> #1301: FILE: hw/pci-host/mv643xx.h:274:
> +#define MV64340_DEVICE_INTERRUPT_MASK^I^I^I^I0x4d4$
> 
> ERROR: code indent should never use tabs
> #1302: FILE: hw/pci-host/mv643xx.h:275:
> +#define MV64340_DEVICE_ERROR_ADDR^I^I^I^I0x4d8$
> 
> ERROR: code indent should never use tabs
> #1303: FILE: hw/pci-host/mv643xx.h:276:
> +#define MV64340_DEVICE_ERROR_DATA   ^I^I^I^I0x4dc$
> 
> ERROR: code indent should never use tabs
> #1304: FILE: hw/pci-host/mv643xx.h:277:
> +#define MV64340_DEVICE_ERROR_PARITY ^I^I^I0x4e0$
> 
>

Re: [RFC v2 11/24] target/arm: kvm: add stubs for some helpers


On 3/1/21 8:49 AM, Claudio Fontana wrote:

--- /dev/null
+++ b/target/arm/kvm/helper-stubs.c
@@ -0,0 +1,18 @@
+#include "qemu/osdep.h"
+#include "cpu.h"


Need license boilerplate.  Otherwise looks fine.


r~

Re: [RFC v2 10/24] target/arm: only perform TCG cpu and machine inits if tcg enabled


On 3/1/21 8:49 AM, Claudio Fontana wrote:

@@ -1321,6 +1323,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
**errp)
  }
  }
  
+#ifdef CONFIG_TCG

  {
  uint64_t scale;
  
@@ -1346,7 +1349,8 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)

  cpu->gt_timer[GTIMER_HYPVIRT] = timer_new(QEMU_CLOCK_VIRTUAL, scale,
arm_gt_hvtimer_cb, cpu);
  }
-#endif
+#endif /* CONFIG_TCG */


You can use tcg_enabled here.


-}
  
  #ifndef CONFIG_USER_ONLY

-cpu->pmu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, arm_pmu_timer_cb,
-cpu);
+cpu->pmu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, arm_pmu_timer_cb,
+  cpu);


Incorrect indentation change.
Otherwise, LGTM.


r~

Re: [PATCH qemu v14] spapr: Implement Open Firmware client interface

On Wed, Feb 24, 2021 at 04:41:30PM +1100, Alexey Kardashevskiy wrote:
> The PAPR platform which describes an OS environment that's presented by
> a combination of a hypervisor and firmware. The features it specifies
> require collaboration between the firmware and the hypervisor.
> 
> Since the beginning, the runtime component of the firmware (RTAS) has
> been implemented as a 20 byte shim which simply forwards it to
> a hypercall implemented in qemu. The boot time firmware component is
> SLOF - but a build that's specific to qemu, and has always needed to be
> updated in sync with it. Even though we've managed to limit the amount
> of runtime communication we need between qemu and SLOF, there's some,
> and it has become increasingly awkward to handle as we've implemented
> new features.
> 
> This implements a boot time OF client interface (CI) which is
> enabled by a new "x-vof" pseries machine option (stands for "Virtual Open
> Firmware). When enabled, QEMU implements the custom H_OF_CLIENT hcall
> which implements Open Firmware Client Interface (OF CI). This allows
> using a smaller stateless firmware which does not have to manage
> the device tree.
> 
> The new "vof.bin" firmware image is included with source code under
> pc-bios/. It also includes RTAS blob.
> 
> This implements a handful of CI methods just to get -kernel/-initrd
> working. In particular, this implements the device tree fetching and
> simple memory allocator - "claim" (an OF CI memory allocator) and updates
> "/memory@0/available" to report the client about available memory.
> 
> This implements changing some device tree properties which we know how
> to deal with, the rest is ignored. To allow changes, this skips
> fdt_pack() when x-vof=on as not packing the blob leaves some room for
> appending.
> 
> In absence of SLOF, this assigns phandles to device tree nodes to make
> device tree traversing work.
> 
> When x-vof=on, this adds "/chosen" every time QEMU (re)builds a tree.
> 
> This adds basic instances support which are managed by a hash map
> ihandle -> [phandle].
> 
> Before the guest started, the used memory is:
> 0..4000 - the initial firmware
> 1..18 - stack
> 
> This OF CI does not implement "interpret".
> 
> Unlike SLOF, this does not format uninitialized nvram. Instead, this
> includes a disk image with pre-formatted nvram.

I think we'll need to improve this, but that can be a later patch.

> With this basic support, this can only boot into kernel directly.
> However this is just enough for the petitboot kernel and initradmdisk to
> boot from any possible source. Note this requires reasonably recent guest
> kernel with:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=df5be5be8735
> 
> The immediate benefit is much faster booting time which especially
> crucial with fully emulated early CPU bring up environments. Also this
> may come handy when/if GRUB-in-the-userspace sees light of the day.
> 
> This separates VOF and sPAPR in a hope that VOF bits may be reused by
> other POWERPC boards which do not support pSeries.
> 
> This is coded in assumption that later on we might be adding support for
> booting from QEMU backends (blockdev is the first candidate) without
> devices/drivers in between as OF1275 does not require that and
> it is quite easy to so.
> 
> Signed-off-by: Alexey Kardashevskiy 
> ---
> 
> The example command line is:
> 
> /home/aik/pbuild/qemu-killslof-localhost-ppc64/qemu-system-ppc64 \
> -nodefaults \
> -chardev stdio,id=STDIO0,signal=off,mux=on \
> -device spapr-vty,id=svty0,reg=0x71000110,chardev=STDIO0 \
> -mon id=MON0,chardev=STDIO0,mode=readline \
> -nographic \
> -vga none \
> -enable-kvm \
> -m 2G \
> -machine 
> pseries,x-vof=on,cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,cap-ccf-assist=off
>  \
> -kernel pbuild/kernel-le-guest/vmlinux \
> -initrd pb/rootfs.cpio.xz \
> -drive 
> id=DRIVE0,if=none,file=./p/qemu-killslof/pc-bios/vof/nvram.bin,format=raw \
> -global spapr-nvram.drive=DRIVE0 \
> -snapshot \
> -smp 8,threads=8 \
> -L /home/aik/t/qemu-ppc64-bios/ \
> -trace events=qemu_trace_events \
> -d guest_errors \
> -chardev socket,id=SOCKET0,server,nowait,path=qemu.mon.tmux26 \
> -mon chardev=SOCKET0,mode=control
> 
> ---
> Changes:
> v14:
> * check for truncates in readstr()
> * ditched a separate vof_reset()
> * spapr->vof is a pointer now, dropped the "on" field
> * removed rtas_base from vof and updated comment why we allow setting it
> * added myself to maintainers
> * updated commit log about blockdev and other possible platforms
> * added a note why new hcall is 0x5
> * no in place endianness convertion in spapr_h_vof_client
> * converted all cpu_physical_memory_read/write to address_space_rw
> * git mv hw/ppc/spapr_vof_client.c hw/ppc/spapr_vof.c
> 
> v13:
> * rebase on latest ppc-for-6.0
> * shuffled code around to touch spapr.c less
> 
> v12:
> * split VOF and SPAPR
> 
> v11:
> * added g_autofree
> * fixed gcc warnings
> * fixed few leaks
> *

[Bug 1917394] [NEW] command lspci does not show the IVSHMEM device

2021-03-01 Thread sean kuo

Public bug reported:

qeum version:
QEMU emulator version 4.2.1

I met a problem when I tried to use IVSHMEM. Command lspci does not show the 
IVSHMEM device.
Below is the configuration from my side:

1.  guest vm xml configuration.
  
  
  2
  


2. after the booting up and I found the qemu commandline ideedly  have the 
device option:
ps aux | grep ivshmem
 /usr/bin/qemu-system-x86_64 
  ...(ignore other options)
-object 
memory-backend-file,id=shmmem-shmem0,mem-path=/dev/shm/hostmem,size=4194304,share=yes
 -device ivshmem-plain,id=shmem0,memdev=shmmem-shmem0,bus=pcie.0,addr=0x10

3. lspci command  not shown the device.

4. lshw command indeedly show the device:

*-memory UNCLAIMED
 description: RAM memory
 product: Inter-VM shared memory
 vendor: Red Hat, Inc.
 physical id: 10
 bus info: pci@:00:10.0
 version: 01
 width: 64 bits
 clock: 33MHz (30.3ns)
 configuration: latency=0
 resources: memory:fcc1c000-fcc1c0ff memory:fdc0-fdff

My host and vm os is ubuntu 20.04 and version is:
#49~20.04.1-Ubuntu SMP Fri Feb 5 09:57:56 UTC 2021 x86_64 x86_64 x86_64 
GNU/Linux

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1917394

Title:
  command lspci does not show the IVSHMEM device

Status in QEMU:
  New

Bug description:
  qeum version:
  QEMU emulator version 4.2.1

  I met a problem when I tried to use IVSHMEM. Command lspci does not show the 
IVSHMEM device.
  Below is the configuration from my side:

  1.  guest vm xml configuration.


2

  

  2. after the booting up and I found the qemu commandline ideedly  have the 
device option:
  ps aux | grep ivshmem
   /usr/bin/qemu-system-x86_64 
...(ignore other options)
  -object 
memory-backend-file,id=shmmem-shmem0,mem-path=/dev/shm/hostmem,size=4194304,share=yes
 -device ivshmem-plain,id=shmem0,memdev=shmmem-shmem0,bus=pcie.0,addr=0x10

  3. lspci command  not shown the device.

  4. lshw command indeedly show the device:

  *-memory UNCLAIMED
   description: RAM memory
   product: Inter-VM shared memory
   vendor: Red Hat, Inc.
   physical id: 10
   bus info: pci@:00:10.0
   version: 01
   width: 64 bits
   clock: 33MHz (30.3ns)
   configuration: latency=0
   resources: memory:fcc1c000-fcc1c0ff memory:fdc0-fdff

  My host and vm os is ubuntu 20.04 and version is:
  #49~20.04.1-Ubuntu SMP Fri Feb 5 09:57:56 UTC 2021 x86_64 x86_64 x86_64 
GNU/Linux

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1917394/+subscriptions

m...@redhat.com

2021-03-01 Thread Sean Kuo

Hi Team,

Sorry for the disturbance.  I have to ask for your help since I can't
solve the problem by googling.

*Below is the problem:*

I met a problem when I tried to use IVSHMEM. Command lspci does not show
the IVSHMEM device.
Below is the configuration from my side:

*1.  guest vm xml configuration.*
  
  
  2
  


*2. after the booting up and I found the qemu commandline ideedly  have the
device option:*
ps aux | grep ivshmem
 /usr/bin/qemu-system-x86_64
  ...(ignore other options)
-object
memory-backend-file,id=shmmem-shmem0,mem-path=/dev/shm/hostmem,size=4194304,share=yes
-device ivshmem-plain,id=shmem0,memdev=shmmem-shmem0,bus=pcie.0,addr=0x10

*3. lspci command  not shown the device.*

*4. lshw command indeedly show the device:*

*-memory UNCLAIMED
 description: RAM memory
 product: Inter-VM shared memory
 vendor: Red Hat, Inc.
 physical id: 10
 bus info: pci@:00:10.0
 version: 01
 width: 64 bits
 clock: 33MHz (30.3ns)
 configuration: latency=0
 resources: memory:fcc1c000-fcc1c0ff memory:fdc0-fdff

My host and vm os is ubuntu 20.04 and version is:
#49~20.04.1-Ubuntu SMP Fri Feb 5 09:57:56 UTC 2021 x86_64 x86_64 x86_64
GNU/Linux

Also I have asked a question on stackoverflow website, I provide the link
for your reference:
https://stackoverflow.com/questions/66432392/comand-lspci-does-not-show-the-the-ivshmem-device

Thanks in advance.

Best regards,
Sean

Re: [PATCH qemu v14] spapr: Implement Open Firmware client interface

On Tue, Feb 23, 2021 at 09:48:56PM -0800, no-re...@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20210224054130.4540-1-...@ozlabs.ru/
> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Type: series
> Message-id: 20210224054130.4540-1-...@ozlabs.ru
> Subject: [PATCH qemu v14] spapr: Implement Open Firmware client interface
> 
> === 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/20210224054130.4540-1-...@ozlabs.ru -> 
> patchew/20210224054130.4540-1-...@ozlabs.ru
> Switched to a new branch 'test'
> 3fc539b spapr: Implement Open Firmware client interface
> 
> === OUTPUT BEGIN ===
> WARNING: line over 80 characters
> #268: FILE: hw/ppc/spapr.c:4463:
> +ClientArchitectureSupportClass *casc = 
> CLIENT_ARCHITECTURE_SUPPORT_CLASS(oc);

These style warnings in the qemu code proper will need to be fixed.

> WARNING: line over 80 characters
> #1431: FILE: hw/ppc/vof.h:29:
> +INTERFACE_CHECK(ClientArchitectureSupport, (obj), 
> TYPE_CLIENT_ARCHITECTURE_SUPPORT)
> 
> ERROR: code indent should never use tabs
> #1548: FILE: pc-bios/vof/bootmem.c:5:
> +^Iuint64_t kern[2];$

I'm a bit torn about these ones in the vof code.  I think it might be
simpler to go to non-tab indenting there (for .c, not .S) just to
avoid checkpatch whining all the time.

Or if you really don't want to update the coding style in VOF, it
would probably be good to include a patch altering checkpatch so it
excludes the VOF code (as it already does for the code imported into
linux-headers).

> 
> ERROR: code indent should never use tabs
> #1549: FILE: pc-bios/vof/bootmem.c:6:
> +^Iphandle chosen = ci_finddevice("/chosen");$
> 
> ERROR: code indent should never use tabs
> #1551: FILE: pc-bios/vof/bootmem.c:8:
> +^Iif (ci_getprop(chosen, "qemu,boot-kernel", kern, sizeof(kern)) !=$
> 
> ERROR: code indent should never use tabs
> #1552: FILE: pc-bios/vof/bootmem.c:9:
> +^I^I^Isizeof(kern))$
> 
> ERROR: code indent should never use tabs
> #1553: FILE: pc-bios/vof/bootmem.c:10:
> +^I^Ireturn;$
> 
> ERROR: code indent should never use tabs
> #1555: FILE: pc-bios/vof/bootmem.c:12:
> +^Ido_boot(kern[0], initrd, initrdsize);$
> 
> ERROR: externs should be avoided in .c files
> #1574: FILE: pc-bios/vof/ci.c:12:
> +extern uint32_t ci_entry(uint32_t params);
> 
> ERROR: externs should be avoided in .c files
> #1576: FILE: pc-bios/vof/ci.c:14:
> +extern unsigned long hv_rtas(unsigned long params);
> 
> ERROR: externs should be avoided in .c files
> #1577: FILE: pc-bios/vof/ci.c:15:
> +extern unsigned int hv_rtas_size;
> 
> ERROR: code indent should never use tabs
> #1581: FILE: pc-bios/vof/ci.c:19:
> +^Ivoid *rtasbase;$
> 
> ERROR: code indent should never use tabs
> #1582: FILE: pc-bios/vof/ci.c:20:
> +^Iuint32_t rtassize = 0;$
> 
> ERROR: code indent should never use tabs
> #1583: FILE: pc-bios/vof/ci.c:21:
> +^Iphandle rtas;$
> 
> ERROR: code indent should never use tabs
> #1585: FILE: pc-bios/vof/ci.c:23:
> +^Iif (strcmp("call-method", (void *)(unsigned long) pargs->service))$
> 
> ERROR: braces {} are necessary for all arms of this statement
> #1585: FILE: pc-bios/vof/ci.c:23:
> +   if (strcmp("call-method", (void *)(unsigned long) pargs->service))
> [...]
> 
> ERROR: code indent should never use tabs
> #1586: FILE: pc-bios/vof/ci.c:24:
> +^I^Ireturn false;$
> 
> ERROR: code indent should never use tabs
> #1588: FILE: pc-bios/vof/ci.c:26:
> +^Iif (strcmp("instantiate-rtas", (void *)(unsigned long) pargs->args[0]))$
> 
> ERROR: braces {} are necessary for all arms of this statement
> #1588: FILE: pc-bios/vof/ci.c:26:
> +   if (strcmp("instantiate-rtas", (void *)(unsigned long) 
> pargs->args[0]))
> [...]
> 
> ERROR: code indent should never use tabs
> #1589: FILE: pc-bios/vof/ci.c:27:
> +^I^Ireturn false;$
> 
> ERROR: code indent should never use tabs
> #1591: FILE: pc-bios/vof/ci.c:29:
> +^Irtas = ci_finddevice("/rtas");$
> 
> ERROR: code indent should never use tabs
> #1592: FILE: pc-bios/vof/ci.c:30:
> +^Ici_getprop(rtas, "rtas-size", , sizeof(rtassize));$
> 
> ERROR: code indent should never use tabs
> #1593: FILE: pc-bios/vof/ci.c:31:
> +^Iif (rtassize < hv_rtas_size)$
> 
> ERROR: braces {} are necessary for all arms of this statement
> #1593: FILE: pc-bios/vof/ci.c:31:
> +   if (rtassize < hv_rtas_size)
> [...]
> 
> ERROR: code indent should never use tabs
> #1594: FILE: pc-bios/vof/ci.c:32:
> +^I^Ireturn false;$
> 
> ERROR: code indent should never use tabs
> #1596: FILE: pc-bios/vof/ci.c:34:
> +^Irtasbase = (void *)(unsigned long) pargs->args[2];$
> 
> ERROR: code indent should never use tabs
>

Re: [PATCH qemu v13] spapr: Implement Open Firmware client interface

On Tue, Feb 23, 2021 at 11:19:38PM +1100, Alexey Kardashevskiy wrote:
> 
> 
> On 23/02/2021 14:07, David Gibson wrote:
> > On Tue, Feb 09, 2021 at 10:02:52PM +1100, Alexey Kardashevskiy wrote:
> > > The PAPR platform which describes an OS environment that's presented by
> > > a combination of a hypervisor and firmware. The features it specifies
> > > require collaboration between the firmware and the hypervisor.
> > > 
> 
> [...]
> 
> > > +target_ulong spapr_h_vof_client(PowerPCCPU *cpu, SpaprMachineState 
> > > *spapr,
> > > +target_ulong opcode, target_ulong *args)
> > > +{
> > > +target_ulong of_client_args = ppc64_phys_to_real(args[0]);
> > > +struct prom_args pargs = { 0 };
> > > +char service[64];
> > > +unsigned nargs, nret, i;
> > > +
> > > +cpu_physical_memory_read(of_client_args, , sizeof(pargs));
> > 
> > Need to check for read errors in case an out of bounds address is passed.
> 
> 
> cpu_physical_memory_read() returns void and so does
> cpu_physical_memory_rw()

Sorry, I'd forgotten that was the case.

> but eventually called address_space_rw() returns an error code, should I
> switch to it?

Yes, I think that would be best.

> > > +nargs = be32_to_cpu(pargs.nargs);
> > > +if (nargs >= ARRAY_SIZE(pargs.args)) {
> > > +return H_PARAMETER;
> > > +}
> > > +
> > > +cpu_physical_memory_read(be32_to_cpu(pargs.service), service,
> > > + sizeof(service));
> > > +if (strnlen(service, sizeof(service)) == sizeof(service)) {
> > > +/* Too long service name */
> > > +return H_PARAMETER;
> > > +}
> > > +
> > > +for (i = 0; i < nargs; ++i) {
> > > +pargs.args[i] = be32_to_cpu(pargs.args[i]);
> > 
> > In general I dislike in-place endian conversion of structs, since I
> > think it's less confusing to think of the endianness as a property of
> > the type.
> 
> The type is uint32_t and there is no be32 in QEMU. I can have 2 copies of
> pargs if this makes reviewing easier, should I?

Even having 2 copies of the struct I don't really like.  Encoding the
endianness down to the individual field level is great when the tools
are available, but as you note qemu doesn't really have that.

But even without that, I like the endianness of structs to be fixed by
convention.  Otherwise when you see a struct instance it's not very
easy to tell if it's a pre-conversion or post-conversion version at
any point in the code.  That means later changes - even just simple
looking code motions can become very fragile, because they move things
to a point where the struct doesn't have the previously expected
endianness.

By preferred solution here when using a struct which needs to map
directly onto in-memory information with a specific endianness is to
*always* leave the struct in that endianness, and only convert when we
actually take things in or out of the struct to use them in
calculations.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature

Re: [PATCH v2 1/1] qemu_timer.c: add timer_deadline_ms() helper

On Mon, Mar 01, 2021 at 09:41:33AM -0300, Daniel Henrique Barboza wrote:
> The pSeries machine is using QEMUTimer internals to return the timeout
> in seconds for a timer object, in hw/ppc/spapr.c, function
> spapr_drc_unplug_timeout_remaining_sec().
> 
> Create a helper in qemu-timer.c to retrieve the deadline for a QEMUTimer
> object, in ms, to avoid exposing timer internals to the PPC code.
> 
> CC: Paolo Bonzini 
> Acked-by: Paolo Bonzini 
> Signed-off-by: Daniel Henrique Barboza 

Applied to ppc-for-6.0, replacing the earlier version.

> ---
>  hw/ppc/spapr_drc.c   |  5 ++---
>  include/qemu/timer.h |  8 
>  util/qemu-timer.c| 13 +
>  3 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 8c4997d795..98b626acf9 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -421,9 +421,8 @@ void spapr_drc_unplug_request(SpaprDrc *drc)
>  
>  int spapr_drc_unplug_timeout_remaining_sec(SpaprDrc *drc)
>  {
> -if (drc->unplug_requested && timer_pending(drc->unplug_timeout_timer)) {
> -return 
> (qemu_timeout_ns_to_ms(drc->unplug_timeout_timer->expire_time) -
> -qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL)) / 1000;
> +if (drc->unplug_requested) {
> +return timer_deadline_ms(drc->unplug_timeout_timer) / 1000;
>  }
>  
>  return 0;
> diff --git a/include/qemu/timer.h b/include/qemu/timer.h
> index 1678238384..5e76e3f8c2 100644
> --- a/include/qemu/timer.h
> +++ b/include/qemu/timer.h
> @@ -795,6 +795,14 @@ static inline int64_t get_max_clock_jump(void)
>  return 60 * NANOSECONDS_PER_SECOND;
>  }
>  
> +/**
> + * timer_deadline_ms:
> + *
> + * Returns the remaining miliseconds for @timer to expire, or zero
> + * if the timer is no longer pending.
> + */
> +int64_t timer_deadline_ms(QEMUTimer *timer);
> +
>  /*
>   * Low level clock functions
>   */
> diff --git a/util/qemu-timer.c b/util/qemu-timer.c
> index 81c28af517..02424bc1b6 100644
> --- a/util/qemu-timer.c
> +++ b/util/qemu-timer.c
> @@ -243,6 +243,19 @@ int64_t timerlist_deadline_ns(QEMUTimerList *timer_list)
>  return delta;
>  }
>  
> +/*
> + * Returns the time remaining for the deadline, in ms.
> + */
> +int64_t timer_deadline_ms(QEMUTimer *timer)
> +{
> +if (timer_pending(timer)) {
> +return qemu_timeout_ns_to_ms(timer->expire_time) -
> +   qemu_clock_get_ms(timer->timer_list->clock->type);
> +}
> +
> +return 0;
> +}
> +
>  /* Calculate the soonest deadline across all timerlists attached
>   * to the clock. This is used for the icount timeout so we
>   * ignore whether or not the clock should be used in deadline

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH 2/5] spapr.c: check unplug_request flag in spapr_memory_unplug_request()

On Fri, Feb 26, 2021 at 01:32:58PM -0300, Daniel Henrique Barboza wrote:
> Now that we're asserting the first DRC LMB earlier, use it to query if
> the DRC is already pending unplug and, in this case, issue the same
> error we already do.
> 
> The previous check was introduced in commit 2a129767ebb1 and it works,
> but it's easier to check the unplug_requested  flag instead of looking
> for the existence of the sPAPRDIMMState. It's also compliant with what
> is already done in other unplug_request functions for other devices.
> 
> Signed-off-by: Daniel Henrique Barboza 

I'm having some trouble completely convincing myself this is right.

What about this situation:
 1. We initiate a DIMM unplug
- unplug_request is set on all the LMBs
- all the LMBs go on the pending_unplug list
 2. The guest encounters no problems, and starts issuing set
indicator calls to mark the LMBs unusable, starting from the
lowest address
 3. On drc_set_unusable() for the first LMB, we see that unplug is
requested and call spapr_drc_release()
 4. spapr_drc_release() on the first LMB clears unplug_requested
 5. At this point, but before this is done on *all* of the DIMM's
LMBs, the user attempts another unplug triggering the code
below

AFAICT this will now skip the error, since the first LMB is no longer
in unplug_requested state, but there *are* still pending unplugs for
some of the remaining LMBs, so the old code would have tripped the
error.

> ---
>  hw/ppc/spapr.c | 8 +---
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 74e046b522..149dc2113f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3681,13 +3681,7 @@ static void spapr_memory_unplug_request(HotplugHandler 
> *hotplug_dev,
>  addr_start / SPAPR_MEMORY_BLOCK_SIZE);
>  g_assert(drc_start);
>  
> -/*
> - * An existing pending dimm state for this DIMM means that there is an
> - * unplug operation in progress, waiting for the spapr_lmb_release
> - * callback to complete the job (BQL can't cover that far). In this case,
> - * bail out to avoid detaching DRCs that were already released.
> - */
> -if (spapr_pending_dimm_unplugs_find(spapr, dimm)) {
> +if (spapr_drc_unplug_requested(drc_start)) {
>  error_setg(errp, "Memory unplug already in progress for device %s",
> dev->id);
>  return;

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH 4/5] spapr_pci.c: add 'unplug already in progress' message for PCI unplug

On Fri, Feb 26, 2021 at 01:33:00PM -0300, Daniel Henrique Barboza wrote:
> Hotunplug for all other devices are warning the user when the hotunplug
> is already in progress. Do the same for PCI devices in
> spapr_pci_unplug_request().
> 
> Signed-off-by: Daniel Henrique Barboza 

Applied to ppc-for-6.0.

> ---
>  hw/ppc/spapr_pci.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index b00e9609ae..feba18cb12 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1743,6 +1743,10 @@ static void spapr_pci_unplug_request(HotplugHandler 
> *plug_handler,
>  }
>  }
>  }
> +} else {
> +error_setg(errp,
> +   "PCI device unplug already in progress for device %s",
> +   drc->dev->id);
>  }
>  }
>  

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH 3/5] spapr.c: add 'unplug already in progress' message for PHB unplug

On Fri, Feb 26, 2021 at 01:32:59PM -0300, Daniel Henrique Barboza wrote:
> Both CPU hotunplug and PC_DIMM unplug reports an user warning,
> mentioning that the hotunplug is in progress, if consecutive
> 'device_del' are issued in quick succession.
> 
> Do the same for PHBs in spapr_phb_unplug_request().
> 
> Signed-off-by: Daniel Henrique Barboza 

LGTM, and doesn't have strong dependencies on the other patches, so
applied to ppc-for-6.0.

> ---
>  hw/ppc/spapr.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 149dc2113f..6ef72ee7bd 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4030,6 +4030,10 @@ static void spapr_phb_unplug_request(HotplugHandler 
> *hotplug_dev,
>  if (!spapr_drc_unplug_requested(drc)) {
>  spapr_drc_unplug_request(drc);
>  spapr_hotplug_req_remove_by_index(drc);
> +} else {
> +error_setg(errp,
> +   "PCI Host Bridge unplug already in progress for device 
> %s",
> +   dev->id);
>  }
>  }
>  

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH 5/5] spapr.c: send QAPI event when memory hotunplug fails

On Fri, Feb 26, 2021 at 01:33:01PM -0300, Daniel Henrique Barboza wrote:
> Recent changes allowed the pSeries machine to rollback the hotunplug
> process for the DIMM when the guest kernel signals, via a
> reconfiguration of the DR connector, that it's not going to release the
> LMBs.
> 
> Let's also warn QAPI listerners about it. One place to do it would be
> right after the unplug state is cleaned up,
> spapr_clear_pending_dimm_unplug_state(). This would mean that the
> function is now doing more than cleaning up the pending dimm state
> though.
> 
> This patch does the following changes in spapr.c:
> 
> - send a QAPI event to inform that we experienced a failure in the
>   hotunplug of the DIMM;
> 
> - rename spapr_clear_pending_dimm_unplug_state() to
>   spapr_memory_unplug_rollback(). This is a better fit for what the
>   function is now doing, and it makes callers care more about what the
>   function goal is and less about spapr.c internals such as clearing
>   the pending dimm unplug state.
> 
> Signed-off-by: Daniel Henrique Barboza 
> ---
>  hw/ppc/spapr.c | 13 +++--
>  hw/ppc/spapr_drc.c |  5 ++---
>  include/hw/ppc/spapr.h |  3 +--
>  3 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 6ef72ee7bd..cbe5cafb14 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -28,6 +28,7 @@
>  #include "qemu-common.h"
>  #include "qemu/datadir.h"
>  #include "qapi/error.h"
> +#include "qapi/qapi-events-machine.h"
>  #include "qapi/visitor.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/hostmem.h"
> @@ -3575,14 +3576,14 @@ 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,
> -   DeviceState *dev)
> +void spapr_memory_unplug_rollback(SpaprMachineState *spapr, DeviceState *dev)
>  {
>  SpaprDimmState *ds;
>  PCDIMMDevice *dimm;
>  SpaprDrc *drc;
>  uint32_t nr_lmbs;
>  uint64_t size, addr_start, addr;
> +g_autofree char *qapi_error = NULL;
>  int i;
>  
>  if (!dev) {
> @@ -3616,6 +3617,14 @@ void 
> spapr_clear_pending_dimm_unplug_state(SpaprMachineState *spapr,
>  drc->unplug_requested = false;
>  addr += SPAPR_MEMORY_BLOCK_SIZE;
>  }
> +
> +/*
> + * Tell QAPI that something happened and the memory
> + * hotunplug wasn't successful.
> + */
> +qapi_error = g_strdup_printf("Memory hotunplug failed for device %s",
> + dev->id);

Might be worth adjusting the error message to make it clearer that it
was the guest which specifically rejected the unplug.  Other than
that, LGTM.

> +qapi_event_send_mem_unplug_error(dev->id, qapi_error);
>  }
>  
>  /* Callback to be called during DRC release. */
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 8c4997d795..8faaf9f1dd 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -1232,12 +1232,11 @@ static void rtas_ibm_configure_connector(PowerPCCPU 
> *cpu,
>  
>  /*
>   * This indicates that the kernel is reconfiguring a LMB due to
> - * a failed hotunplug. Clear the pending unplug state for the whole
> - * DIMM.
> + * a failed hotunplug. Rollback the DIMM unplug process.
>   */
>  if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_LMB &&
>  drc->unplug_requested) {
> -spapr_clear_pending_dimm_unplug_state(spapr, drc->dev);
> +spapr_memory_unplug_rollback(spapr, drc->dev);
>  }
>  
>  if (!drc->fdt) {
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index d6edeaaaff..47cebaf3ac 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -847,8 +847,7 @@ 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,
> -   DeviceState *dev);
> +void spapr_memory_unplug_rollback(SpaprMachineState *spapr, DeviceState 
> *dev);
>  int spapr_max_server_number(SpaprMachineState *spapr);
>  void spapr_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
>uint64_t pte0, uint64_t pte1);

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

[PATCH] file-posix: allow -EBUSY errors during write zeros on block

2021-03-01 Thread ChangLimin

After Linux 5.10, write zeros to a multipath device using
ioctl(fd, BLKZEROOUT, range) with cache none or directsync will return EBUSY.

Similar to handle_aiocb_write_zeroes_unmap, handle_aiocb_write_zeroes_block
allow -EBUSY errors during ioctl(fd, BLKZEROOUT, range).

Reference commit in Linux 5.10:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=384d87ef2c954fc58e6c5fd8253e4a1984f5fe02

Signed-off-by: ChangLimin 
---
 block/file-posix.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 05079b40ca..3e60c96214 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1629,8 +1629,13 @@ static ssize_t 
handle_aiocb_write_zeroes_block(RawPosixAIOData *aiocb)
         } while (errno == EINTR);

         ret = translate_err(-errno);
-        if (ret == -ENOTSUP) {
+        switch (ret) {
+        case -ENOTSUP:
+        case -EINVAL:
+        case -EBUSY:
             s->has_write_zeroes = false;
+            return -ENOTSUP;
+            break;
         }
     }
 #endif
--
2.27.0

[PATCH] block: Change write_threshold to uint64 in BlockDeviceInfo

2021-03-01 Thread Yi Wang

From: renlei4 

write_threshold is saved as uint64, but BlockDeviceInfo use int to describe it.

normally it works well if threshold less than max int:
 # virsh domblkthreshold v6_163 sda 9223372036854775807
 # virsh qemu-monitor-command v6_163  '{ "execute": "query-named-block-nodes" 
}' |grep threshold
"write_threshold": 9223372036854775807,

overflow happened if threshold greater than max int:
 # virsh domblkthreshold v6_163 sda 9223372036854775811
 # virsh qemu-monitor-command v6_163   '{ "execute": "query-named-block-nodes" 
}' |grep threshold
"write_threshold": -9223372036854775805,

Fixes: e2462113b200 "block: add event when disk usage exceeds threshold"

Signed-off-by: Ren Lei 
Signed-off-by: Yi Wang 
---
 qapi/block-core.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9f555d5..00b8729 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -409,7 +409,7 @@
 '*bps_wr_max_length': 'int', '*iops_max_length': 'int',
 '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
 '*iops_size': 'int', '*group': 'str', 'cache': 'BlockdevCacheInfo',
-'write_threshold': 'int', '*dirty-bitmaps': ['BlockDirtyInfo'] } }
+'write_threshold': 'uint64', '*dirty-bitmaps': ['BlockDirtyInfo'] 
} }
 
 ##
 # @BlockDeviceIoStatus:
-- 
1.8.3.1

[PATCH v3 5/5] hw/misc: Model KCS devices in the Aspeed LPC controller

Keyboard-Controller-Style devices for IPMI purposes are exposed via LPC
IO cycles from the BMC to the host.

Expose support on the BMC side by implementing the usual MMIO
behaviours, and expose the ability to inspect the KCS registers in
"host" style by accessing QOM properties associated with each register.

The model caters to the IRQ style of both the AST2600 and the earlier
SoCs (AST2400 and AST2500). The AST2600 allocates an IRQ for each LPC
sub-device, while there is a single IRQ shared across all subdevices on
the AST2400 and AST2500.

Signed-off-by: Andrew Jeffery 
Reviewed-by: Cédric Le Goater 
---
 hw/arm/aspeed_ast2600.c  |  28 ++-
 hw/arm/aspeed_soc.c  |  24 ++-
 hw/misc/aspeed_lpc.c | 359 ++-
 include/hw/arm/aspeed_soc.h  |   1 +
 include/hw/misc/aspeed_lpc.h |  17 +-
 5 files changed, 424 insertions(+), 5 deletions(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 5a7b8ba81c92..4f83097e4a26 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -104,7 +104,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
 [ASPEED_DEV_ETH2]  = 3,
 [ASPEED_DEV_ETH3]  = 32,
 [ASPEED_DEV_ETH4]  = 33,
-
+[ASPEED_DEV_KCS]   = 138,   /* 138 -> 142 */
 };
 
 static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
@@ -477,8 +477,34 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 return;
 }
 sysbus_mmio_map(SYS_BUS_DEVICE(>lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
+
+/* Connect the LPC IRQ to the GIC. It is otherwise unused. */
 sysbus_connect_irq(SYS_BUS_DEVICE(>lpc), 0,
aspeed_soc_get_irq(s, ASPEED_DEV_LPC));
+
+/*
+ * On the AST2600 LPC subdevice IRQs are connected straight to the GIC.
+ *
+ * LPC subdevice IRQ sources are offset from 1 because the LPC model caters
+ * to the AST2400 and AST2500. SoCs before the AST2600 have one LPC IRQ
+ * shared across the subdevices, and the shared IRQ output to the VIC is at
+ * offset 0.
+ */
+sysbus_connect_irq(SYS_BUS_DEVICE(>lpc), 1 + aspeed_lpc_kcs_1,
+   qdev_get_gpio_in(DEVICE(>a7mpcore),
+sc->irqmap[ASPEED_DEV_KCS] + 
aspeed_lpc_kcs_1));
+
+sysbus_connect_irq(SYS_BUS_DEVICE(>lpc), 1 + aspeed_lpc_kcs_2,
+   qdev_get_gpio_in(DEVICE(>a7mpcore),
+sc->irqmap[ASPEED_DEV_KCS] + 
aspeed_lpc_kcs_2));
+
+sysbus_connect_irq(SYS_BUS_DEVICE(>lpc), 1 + aspeed_lpc_kcs_3,
+   qdev_get_gpio_in(DEVICE(>a7mpcore),
+sc->irqmap[ASPEED_DEV_KCS] + 
aspeed_lpc_kcs_3));
+
+sysbus_connect_irq(SYS_BUS_DEVICE(>lpc), 1 + aspeed_lpc_kcs_4,
+   qdev_get_gpio_in(DEVICE(>a7mpcore),
+sc->irqmap[ASPEED_DEV_KCS] + 
aspeed_lpc_kcs_4));
 }
 
 static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 4f098da437ac..057d053c8478 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -112,7 +112,6 @@ static const int aspeed_soc_ast2400_irqmap[] = {
 [ASPEED_DEV_WDT]= 27,
 [ASPEED_DEV_PWM]= 28,
 [ASPEED_DEV_LPC]= 8,
-[ASPEED_DEV_IBT]= 8, /* LPC */
 [ASPEED_DEV_I2C]= 12,
 [ASPEED_DEV_ETH1]   = 2,
 [ASPEED_DEV_ETH2]   = 3,
@@ -401,8 +400,31 @@ static void aspeed_soc_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 sysbus_mmio_map(SYS_BUS_DEVICE(>lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
+
+/* Connect the LPC IRQ to the VIC */
 sysbus_connect_irq(SYS_BUS_DEVICE(>lpc), 0,
aspeed_soc_get_irq(s, ASPEED_DEV_LPC));
+
+/*
+ * On the AST2400 and AST2500 the one LPC IRQ is shared between all of the
+ * subdevices. Connect the LPC subdevice IRQs to the LPC controller IRQ (by
+ * contrast, on the AST2600, the subdevice IRQs are connected straight to
+ * the GIC).
+ *
+ * LPC subdevice IRQ sources are offset from 1 because the shared IRQ 
output
+ * to the VIC is at offset 0.
+ */
+sysbus_connect_irq(SYS_BUS_DEVICE(>lpc), 1 + aspeed_lpc_kcs_1,
+   qdev_get_gpio_in(DEVICE(>lpc), aspeed_lpc_kcs_1));
+
+sysbus_connect_irq(SYS_BUS_DEVICE(>lpc), 1 + aspeed_lpc_kcs_2,
+   qdev_get_gpio_in(DEVICE(>lpc), aspeed_lpc_kcs_2));
+
+sysbus_connect_irq(SYS_BUS_DEVICE(>lpc), 1 + aspeed_lpc_kcs_3,
+   qdev_get_gpio_in(DEVICE(>lpc), aspeed_lpc_kcs_3));
+
+sysbus_connect_irq(SYS_BUS_DEVICE(>lpc), 1 + aspeed_lpc_kcs_4,
+   qdev_get_gpio_in(DEVICE(>lpc), aspeed_lpc_kcs_4));
 }
 static Property aspeed_soc_properties[] = {
 DEFINE_PROP_LINK("dram", AspeedSoCState, dram_mr, TYPE_MEMORY_REGION,
diff --git a/hw/misc/aspeed_lpc.c b/hw/misc/aspeed_lpc.c
index e668e985ff04..2dddb27c35d0

[PATCH v3 4/5] hw/misc: Add a basic Aspeed LPC controller model

From: Cédric Le Goater 

This is a very minimal framework to access registers which are used to
configure the AHB memory mapping of the flash chips on the LPC HC
Firmware address space.

Signed-off-by: Cédric Le Goater 
Signed-off-by: Andrew Jeffery 
---
 docs/system/arm/aspeed.rst   |   2 +-
 hw/arm/aspeed_ast2600.c  |  10 +++
 hw/arm/aspeed_soc.c  |  10 +++
 hw/misc/aspeed_lpc.c | 131 +++
 hw/misc/meson.build  |   7 +-
 include/hw/arm/aspeed_soc.h  |   2 +
 include/hw/misc/aspeed_lpc.h |  32 +
 7 files changed, 192 insertions(+), 2 deletions(-)
 create mode 100644 hw/misc/aspeed_lpc.c
 create mode 100644 include/hw/misc/aspeed_lpc.h

diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
index 690bada7842b..2f6fa8938d02 100644
--- a/docs/system/arm/aspeed.rst
+++ b/docs/system/arm/aspeed.rst
@@ -48,6 +48,7 @@ Supported devices
  * UART
  * Ethernet controllers
  * Front LEDs (PCA9552 on I2C bus)
+ * LPC Peripheral Controller (a subset of subdevices are supported)
 
 
 Missing devices
@@ -56,7 +57,6 @@ Missing devices
  * Coprocessor support
  * ADC (out of tree implementation)
  * PWM and Fan Controller
- * LPC Bus Controller
  * Slave GPIO Controller
  * Super I/O Controller
  * Hash/Crypto Engine
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 2125a96ef317..5a7b8ba81c92 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -211,6 +211,8 @@ static void aspeed_soc_ast2600_init(Object *obj)
 
 object_initialize_child(obj, "emmc-controller.sdhci", >emmc.slots[0],
 TYPE_SYSBUS_SDHCI);
+
+object_initialize_child(obj, "lpc", >lpc, TYPE_ASPEED_LPC);
 }
 
 /*
@@ -469,6 +471,14 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 sysbus_mmio_map(SYS_BUS_DEVICE(>emmc), 0, sc->memmap[ASPEED_DEV_EMMC]);
 sysbus_connect_irq(SYS_BUS_DEVICE(>emmc), 0,
aspeed_soc_get_irq(s, ASPEED_DEV_EMMC));
+
+/* LPC */
+if (!sysbus_realize(SYS_BUS_DEVICE(>lpc), errp)) {
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(>lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
+sysbus_connect_irq(SYS_BUS_DEVICE(>lpc), 0,
+   aspeed_soc_get_irq(s, ASPEED_DEV_LPC));
 }
 
 static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 7eefd54ac07a..4f098da437ac 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -211,6 +211,8 @@ static void aspeed_soc_init(Object *obj)
 object_initialize_child(obj, "sdhci[*]", >sdhci.slots[i],
 TYPE_SYSBUS_SDHCI);
 }
+
+object_initialize_child(obj, "lpc", >lpc, TYPE_ASPEED_LPC);
 }
 
 static void aspeed_soc_realize(DeviceState *dev, Error **errp)
@@ -393,6 +395,14 @@ static void aspeed_soc_realize(DeviceState *dev, Error 
**errp)
 sc->memmap[ASPEED_DEV_SDHCI]);
 sysbus_connect_irq(SYS_BUS_DEVICE(>sdhci), 0,
aspeed_soc_get_irq(s, ASPEED_DEV_SDHCI));
+
+/* LPC */
+if (!sysbus_realize(SYS_BUS_DEVICE(>lpc), errp)) {
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(>lpc), 0, sc->memmap[ASPEED_DEV_LPC]);
+sysbus_connect_irq(SYS_BUS_DEVICE(>lpc), 0,
+   aspeed_soc_get_irq(s, ASPEED_DEV_LPC));
 }
 static Property aspeed_soc_properties[] = {
 DEFINE_PROP_LINK("dram", AspeedSoCState, dram_mr, TYPE_MEMORY_REGION,
diff --git a/hw/misc/aspeed_lpc.c b/hw/misc/aspeed_lpc.c
new file mode 100644
index ..e668e985ff04
--- /dev/null
+++ b/hw/misc/aspeed_lpc.c
@@ -0,0 +1,131 @@
+/*
+ *  ASPEED LPC Controller
+ *
+ *  Copyright (C) 2017-2018 IBM Corp.
+ *
+ * This code is licensed under the GPL version 2 or later.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/error-report.h"
+#include "hw/misc/aspeed_lpc.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+
+#define TO_REG(offset) ((offset) >> 2)
+
+#define HICR0TO_REG(0x00)
+#define HICR1TO_REG(0x04)
+#define HICR2TO_REG(0x08)
+#define HICR3TO_REG(0x0C)
+#define HICR4TO_REG(0x10)
+#define HICR5TO_REG(0x80)
+#define HICR6TO_REG(0x84)
+#define HICR7TO_REG(0x88)
+#define HICR8TO_REG(0x8C)
+
+static uint64_t aspeed_lpc_read(void *opaque, hwaddr offset, unsigned size)
+{
+AspeedLPCState *s = ASPEED_LPC(opaque);
+int reg = TO_REG(offset);
+
+if (reg >= ARRAY_SIZE(s->regs)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: Out-of-bounds read at offset 0x%" HWADDR_PRIx "\n",
+  __func__, offset);
+return 0;
+}
+
+return s->regs[reg];
+}
+
+static void aspeed_lpc_write(void *opaque, hwaddr offset,

[PATCH v3 1/5] hw/arm: ast2600: Force a multiple of 32 of IRQs for the GIC

This appears to be a requirement of the GIC model. The AST2600 allocates
197 GIC IRQs, which we will adjust shortly.

Signed-off-by: Andrew Jeffery 
Reviewed-by: Cédric Le Goater 
---
 hw/arm/aspeed_ast2600.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index bf31ca351feb..bc0eeb058b24 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -65,7 +65,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
 
 #define ASPEED_A7MPCORE_ADDR 0x4046
 
-#define ASPEED_SOC_AST2600_MAX_IRQ 128
+#define AST2600_MAX_IRQ 128
 
 /* Shared Peripheral Interrupt values below are offset by -32 from datasheet */
 static const int aspeed_soc_ast2600_irqmap[] = {
@@ -267,7 +267,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 object_property_set_int(OBJECT(>a7mpcore), "num-cpu", sc->num_cpus,
 _abort);
 object_property_set_int(OBJECT(>a7mpcore), "num-irq",
-ASPEED_SOC_AST2600_MAX_IRQ + GIC_INTERNAL,
+ROUND_UP(AST2600_MAX_IRQ + GIC_INTERNAL, 32),
 _abort);
 
 sysbus_realize(SYS_BUS_DEVICE(>a7mpcore), _abort);
-- 
2.27.0

[PATCH v3 3/5] hw/arm: ast2600: Correct the iBT interrupt ID

The AST2600 allocates distinct GIC IRQs for the LPC subdevices such as
the iBT device. Previously on the AST2400 and AST2500 the LPC subdevices
shared a single LPC IRQ.

Signed-off-by: Andrew Jeffery 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cédric Le Goater 
---
 hw/arm/aspeed_ast2600.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 22fcb5b0edbe..2125a96ef317 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -98,7 +98,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
 [ASPEED_DEV_WDT]   = 24,
 [ASPEED_DEV_PWM]   = 44,
 [ASPEED_DEV_LPC]   = 35,
-[ASPEED_DEV_IBT]   = 35,/* LPC */
+[ASPEED_DEV_IBT]   = 143,
 [ASPEED_DEV_I2C]   = 110,   /* 110 -> 125 */
 [ASPEED_DEV_ETH1]  = 2,
 [ASPEED_DEV_ETH2]  = 3,
-- 
2.27.0

[PATCH v3 2/5] hw/arm: ast2600: Set AST2600_MAX_IRQ to value from datasheet

The datasheet says we have 197 IRQs allocated, and we need more than 128
to describe IRQs from LPC devices. Raise the value now to allow
modelling of the LPC devices.

Signed-off-by: Andrew Jeffery 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cédric Le Goater 
---
 hw/arm/aspeed_ast2600.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index bc0eeb058b24..22fcb5b0edbe 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -65,7 +65,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
 
 #define ASPEED_A7MPCORE_ADDR 0x4046
 
-#define AST2600_MAX_IRQ 128
+#define AST2600_MAX_IRQ 197
 
 /* Shared Peripheral Interrupt values below are offset by -32 from datasheet */
 static const int aspeed_soc_ast2600_irqmap[] = {
-- 
2.27.0

[PATCH v3 0/5] aspeed: LPC peripheral controller devices

Hello,

This series adds support for some of the LPC[1] peripherals found in Aspeed BMC
SoCs.

[1]
https://www.intel.com/content/dam/www/program/design/us/en/documents/low-pin-count-interface-specification.pdf

v3 fixes a copy/paste error hooking up the LPC IRQ for the AST2600, identified
off-list. I've tested exercised the eMMC path to confirm the fix. v2 of the
series can be found here:

https://lore.kernel.org/qemu-devel/20210301010610.355702-1-and...@aj.id.au/T/#mccf00fea21d955d74de39dbc49af8451b447ff54

BMCs typically provide a number of features to their host via LPC that include
but are not limited to:

1. Mapping LPC firmware cycles to BMC-controlled flash devices
2. UART(s) for system console routing
3. POST code routing
4. Keyboard-Controller-Style (KCS) IPMI devices
5. Block Transfer (BT) IPMI devices
6. A SuperIO controller for management of LPC devices and miscellaneous
functionality

Specifically, this series adds basic support for functions 1 and 4 above,
handling the BMC firmware configuring the bridge mapping LPC firmware cycles
onto its AHB as well as support for four KCS devices.

Aspeed's LPC controller is not a straight-forward device by any stretch. It
contains at least the capabilities outlined above, in the sense that it's not
possible to cleanly separate the different functions into distinct MMIO
sub-regions: Registers for the various bits of functionality have the feel of
arbitrary placement with a nod to feature-creep and backwards compatibility.
Further, the conceptually coherent pieces of functionality often come with the
ability to issue interrupts, though for the AST2400 and AST2500 there is one
shared VIC IRQ for all LPC "subdevices". By contrast the AST2600 gives each
subdevice a distinct IRQ via the GIC.

All this combined leads to some complexity regarding the interrupts and handling
the MMIO accesses (in terms of mapping the access back to the function it's
affecting).

Finally, as a point of clarity, Aspeed BMCs also contain an LPC Host Controller
to drive the LPC bus. This series does not concern itself with the LPC Host
Controller function, only with a subset of the peripheral devices the BMC
presents to the host.

I've tested the series using a combination of the ast2600-evb, witherspoon-bmc
and romulus-bmc machines along with a set of recently-posted patches for
Linux[2].

Please review!

Andrew

[2]
https://lore.kernel.org/openbmc/20210219142523.3464540-1-and...@aj.id.au/T/#m1e2029e7aa2be3056320e8d46b3b5b1539a776b4

Andrew Jeffery (4):
hw/arm: ast2600: Force a multiple of 32 of IRQs for the GIC
hw/arm: ast2600: Set AST2600_MAX_IRQ to value from datasheet
hw/arm: ast2600: Correct the iBT interrupt ID
hw/misc: Model KCS devices in the Aspeed LPC controller

Cédric Le Goater (1):
hw/misc: Add a basic Aspeed LPC controller model

docs/system/arm/aspeed.rst | 2 +-
hw/arm/aspeed_ast2600.c | 44 +++-
hw/arm/aspeed_soc.c | 34 ++-
hw/misc/aspeed_lpc.c | 486 +++
hw/misc/meson.build | 7 +-
include/hw/arm/aspeed_soc.h | 3 +
include/hw/misc/aspeed_lpc.h | 47
7 files changed, 616 insertions(+), 7 deletions(-)
create mode 100644 hw/misc/aspeed_lpc.c
create mode 100644 include/hw/misc/aspeed_lpc.h

base-commit: 51db2d7cf26d05a961ec0ee0eb773594b32cc4a1
--
2.27.0

[PATCH 2/2] blockdev: Clarify error messages pertaining to 'node-name'

2021-03-01 Thread Connor Kuehl

Signed-off-by: Connor Kuehl 
---
 blockdev.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index cd438e60e3..7c7ab2b386 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1515,13 +1515,13 @@ static void external_snapshot_prepare(BlkActionState 
*common,
 s->has_snapshot_node_name ? s->snapshot_node_name : NULL;
 
 if (node_name && !snapshot_node_name) {
-error_setg(errp, "New overlay node name missing");
+error_setg(errp, "New overlay node-name missing");
 goto out;
 }
 
 if (snapshot_node_name &&
 bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) {
-error_setg(errp, "New overlay node name already in use");
+error_setg(errp, "New overlay node-name already in use");
 goto out;
 }
 
@@ -3598,13 +3598,14 @@ void qmp_x_blockdev_reopen(BlockdevOptions *options, 
Error **errp)
 
 /* Check for the selected node name */
 if (!options->has_node_name) {
-error_setg(errp, "Node name not specified");
+error_setg(errp, "node-name not specified");
 goto fail;
 }
 
 bs = bdrv_find_node(options->node_name);
 if (!bs) {
-error_setg(errp, "Cannot find node named '%s'", options->node_name);
+error_setg(errp, "Failed to find node with node-name='%s'",
+   options->node_name);
 goto fail;
 }
 
@@ -3635,7 +3636,7 @@ void qmp_blockdev_del(const char *node_name, Error **errp)
 
 bs = bdrv_find_node(node_name);
 if (!bs) {
-error_setg(errp, "Cannot find node %s", node_name);
+error_setg(errp, "Failed to find node with node-name='%s'", node_name);
 return;
 }
 if (bdrv_has_blk(bs)) {
@@ -3758,7 +3759,7 @@ void qmp_x_blockdev_set_iothread(const char *node_name, 
StrOrNull *iothread,
 
 bs = bdrv_find_node(node_name);
 if (!bs) {
-error_setg(errp, "Cannot find node %s", node_name);
+error_setg(errp, "Failed to find node with node-name='%s'", node_name);
 return;
 }
 
-- 
2.29.2

[PATCH 0/2] Clarify error messages pertaining to 'node-name'

2021-03-01 Thread Connor Kuehl

Some error messages contain ambiguous representations of the 'node-name'
parameter. This can be particularly confusing when exchanging QMP
messages (C = client, S = server):

C: {"execute": "block_resize", "arguments": { "device": "my_file", "size": 
26843545600 }}
S: {"error": {"class": "GenericError", "desc": "Cannot find device=my_file nor 
node_name="}}
   
^

This error message suggests one could send a message with a key called
'node_name':

C: {"execute": "block_resize", "arguments": { "node_name": "my_file", "size": 
26843545600 }}
   ^

but using the underscore is actually incorrect, the parameter should be
'node-name':

S: {"error": {"class": "GenericError", "desc": "Parameter 'node_name' is 
unexpected"}}

This behavior was uncovered in bz1651437[1], but I ended up going down a
rabbit hole looking for other areas where this miscommunication might
occur and changing those accordingly as well.

[1] https://bugzilla.redhat.com/1651437

Connor Kuehl (2):
  block: Clarify error messages pertaining to 'node-name'
  blockdev: Clarify error messages pertaining to 'node-name'

 block.c|  8 
 blockdev.c | 13 +++--
 tests/qemu-iotests/040 |  4 ++--
 tests/qemu-iotests/249.out |  2 +-
 4 files changed, 14 insertions(+), 13 deletions(-)

-- 
2.29.2

[PATCH 1/2] block: Clarify error messages pertaining to 'node-name'

2021-03-01 Thread Connor Kuehl

Reported-by: Tingting Mao 
Fixes: https://bugzilla.redhat.com/1651437
Signed-off-by: Connor Kuehl 
---
 block.c| 8 
 tests/qemu-iotests/040 | 4 ++--
 tests/qemu-iotests/249.out | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index a1f3cecd75..2daff6d29a 100644
--- a/block.c
+++ b/block.c
@@ -1440,7 +1440,7 @@ static void bdrv_assign_node_name(BlockDriverState *bs,
  * Check for empty string or invalid characters, but not if it is
  * generated (generated names use characters not available to the user)
  */
-error_setg(errp, "Invalid node name");
+error_setg(errp, "Invalid node-name: '%s'", node_name);
 return;
 }
 
@@ -1453,7 +1453,7 @@ static void bdrv_assign_node_name(BlockDriverState *bs,
 
 /* takes care of avoiding duplicates node names */
 if (bdrv_find_node(node_name)) {
-error_setg(errp, "Duplicate node name");
+error_setg(errp, "Duplicate nodes with node-name='%s'", node_name);
 goto out;
 }
 
@@ -5432,7 +5432,7 @@ BlockDriverState *bdrv_lookup_bs(const char *device,
 }
 }
 
-error_setg(errp, "Cannot find device=%s nor node_name=%s",
+error_setg(errp, "Cannot find device=\'%s\' nor node-name=\'%s\'",
  device ? device : "",
  node_name ? node_name : "");
 return NULL;
@@ -6752,7 +6752,7 @@ BlockDriverState *check_to_replace_node(BlockDriverState 
*parent_bs,
 AioContext *aio_context;
 
 if (!to_replace_bs) {
-error_setg(errp, "Node name '%s' not found", node_name);
+error_setg(errp, "Failed to find node with node-name='%s'", node_name);
 return NULL;
 }
 
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 7ebc9ed825..336ff7c4f2 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -175,13 +175,13 @@ class TestSingleDrive(ImageCommitTestCase):
 self.assert_no_active_block_jobs()
 result = self.vm.qmp('block-commit', device='drive0', 
top_node='badfile', base_node='base')
 self.assert_qmp(result, 'error/class', 'GenericError')
-self.assert_qmp(result, 'error/desc', "Cannot find device= nor 
node_name=badfile")
+self.assert_qmp(result, 'error/desc', "Cannot find device='' nor 
node-name='badfile'")
 
 def test_base_node_invalid(self):
 self.assert_no_active_block_jobs()
 result = self.vm.qmp('block-commit', device='drive0', top_node='mid', 
base_node='badfile')
 self.assert_qmp(result, 'error/class', 'GenericError')
-self.assert_qmp(result, 'error/desc', "Cannot find device= nor 
node_name=badfile")
+self.assert_qmp(result, 'error/desc', "Cannot find device='' nor 
node-name='badfile'")
 
 def test_top_path_and_node(self):
 self.assert_no_active_block_jobs()
diff --git a/tests/qemu-iotests/249.out b/tests/qemu-iotests/249.out
index 92ec81db03..d2bf9be85e 100644
--- a/tests/qemu-iotests/249.out
+++ b/tests/qemu-iotests/249.out
@@ -18,7 +18,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 
backing_file=TEST_DIR/t.
  'filter-node-name': '1234'}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"JOB_STATUS_CHANGE", "data": {"status": "created", "id": "job0"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"JOB_STATUS_CHANGE", "data": {"status": "null", "id": "job0"}}
-{"error": {"class": "GenericError", "desc": "Invalid node name"}}
+{"error": {"class": "GenericError", "desc": "Invalid node-name: '1234'"}}
 
 === Send a write command to a drive opened in read-only mode (2)
 
-- 
2.29.2

Re: [RFC v2 08/24] target/arm: split cpregs from tcg/helper.c


On 3/1/21 8:49 AM, Claudio Fontana wrote:

+uint64_t raw_read(CPUARMState *env, const ARMCPRegInfo *ri);
+void raw_write(CPUARMState *env, const ARMCPRegInfo *ri,
+   uint64_t value);


These shouldn't go in cpu.h; the new cpregs.h seems reasonable.

Otherwise, LGTM.

r~

Re: [PATCH v2 16/42] esp: use pdma_origin directly in esp_pdma_read()/esp_pdma_write()

Le 09/02/2021 à 20:29, Mark Cave-Ayland a écrit :
> This is the first step in removing get_pdma_buf() from esp.c.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/scsi/esp.c | 34 --
>  1 file changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index b0cba889a9..cfeba2feb0 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -153,16 +153,38 @@ static uint8_t *get_pdma_buf(ESPState *s)
>  
>  static uint8_t esp_pdma_read(ESPState *s)
>  {
> -uint8_t *buf = get_pdma_buf(s);
> -
> -return buf[s->pdma_cur++];
> +switch (s->pdma_origin) {
> +case PDMA:
> +return s->pdma_buf[s->pdma_cur++];
> +case TI:
> +return s->ti_buf[s->pdma_cur++];
> +case CMD:
> +return s->cmdbuf[s->pdma_cur++];
> +case ASYNC:
> +return s->async_buf[s->pdma_cur++];
> +default:
> +g_assert_not_reached();
> +}
>  }
>  
>  static void esp_pdma_write(ESPState *s, uint8_t val)
>  {
> -uint8_t *buf = get_pdma_buf(s);
> -
> -buf[s->pdma_cur++] = val;
> +switch (s->pdma_origin) {
> +case PDMA:
> +s->pdma_buf[s->pdma_cur++] = val;
> +break;
> +case TI:
> +s->ti_buf[s->pdma_cur++] = val;
> +break;
> +case CMD:
> +s->cmdbuf[s->pdma_cur++] = val;
> +break;
> +case ASYNC:
> +s->async_buf[s->pdma_cur++] = val;
> +break;
> +default:
> +g_assert_not_reached();
> +}
>  }
>  
>  static int get_cmd_cb(ESPState *s)
> 

Reviewed-by: Laurent Vivier

Re: [PATCH v2 15/42] esp: introduce esp_pdma_read() and esp_pdma_write() functions

Le 09/02/2021 à 20:29, Mark Cave-Ayland a écrit :
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/scsi/esp.c | 28 
>  1 file changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index e7cf36f4b8..b0cba889a9 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -151,6 +151,20 @@ static uint8_t *get_pdma_buf(ESPState *s)
>  return NULL;
>  }
>  
> +static uint8_t esp_pdma_read(ESPState *s)
> +{
> +uint8_t *buf = get_pdma_buf(s);
> +
> +return buf[s->pdma_cur++];
> +}
> +
> +static void esp_pdma_write(ESPState *s, uint8_t val)
> +{
> +uint8_t *buf = get_pdma_buf(s);
> +
> +buf[s->pdma_cur++] = val;
> +}
> +
>  static int get_cmd_cb(ESPState *s)
>  {
>  int target;
> @@ -910,7 +924,6 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr 
> addr,
>  SysBusESPState *sysbus = opaque;
>  ESPState *s = ESP(>esp);
>  uint32_t dmalen = esp_get_tc(s);
> -uint8_t *buf = get_pdma_buf(s);
>  
>  trace_esp_pdma_write(size);
>  
> @@ -919,13 +932,13 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr 
> addr,
>  }
>  switch (size) {
>  case 1:
> -buf[s->pdma_cur++] = val;
> +esp_pdma_write(s, val);
>  s->pdma_len--;
>  dmalen--;
>  break;
>  case 2:
> -buf[s->pdma_cur++] = val >> 8;
> -buf[s->pdma_cur++] = val;
> +esp_pdma_write(s, val >> 8);
> +esp_pdma_write(s, val);
>  s->pdma_len -= 2;
>  dmalen -= 2;
>  break;
> @@ -944,7 +957,6 @@ static uint64_t sysbus_esp_pdma_read(void *opaque, hwaddr 
> addr,
>  SysBusESPState *sysbus = opaque;
>  ESPState *s = ESP(>esp);
>  uint32_t dmalen = esp_get_tc(s);
> -uint8_t *buf = get_pdma_buf(s);
>  uint64_t val = 0;
>  
>  trace_esp_pdma_read(size);
> @@ -954,13 +966,13 @@ static uint64_t sysbus_esp_pdma_read(void *opaque, 
> hwaddr addr,
>  }
>  switch (size) {
>  case 1:
> -val = buf[s->pdma_cur++];
> +val = esp_pdma_read(s);
>  s->pdma_len--;
>  dmalen--;
>  break;
>  case 2:
> -val = buf[s->pdma_cur++];
> -val = (val << 8) | buf[s->pdma_cur++];
> +val = esp_pdma_read(s);
> +val = (val << 8) | esp_pdma_read(s);
>  s->pdma_len -= 2;
>  dmalen -= 2;
>  break;
> 

Reviewed-by: Laurent Vivier

[PATCH v2 15/17] cpu: Move CPUClass::get_paging_enabled to SysemuCPUOps

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h| 2 --
 include/hw/core/sysemu-cpu-ops.h | 4 
 hw/core/cpu.c| 4 ++--
 target/i386/cpu.c| 4 +++-
 4 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 8af78cdde23..960846d2b64 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -93,7 +93,6 @@ struct AccelCPUClass;
  * @dump_state: Callback for dumping state.
  * @dump_statistics: Callback for dumping statistics.
  * @get_arch_id: Callback for getting architecture-dependent CPU ID.
- * @get_paging_enabled: Callback for inquiring whether paging is enabled.
  * @set_pc: Callback for setting the Program Counter register. This
  *   should have the semantics used by the target architecture when
  *   setting the PC from a source such as an ELF file entry point;
@@ -136,7 +135,6 @@ struct CPUClass {
 void (*dump_state)(CPUState *cpu, FILE *, int flags);
 void (*dump_statistics)(CPUState *cpu, int flags);
 int64_t (*get_arch_id)(CPUState *cpu);
-bool (*get_paging_enabled)(const CPUState *cpu);
 void (*set_pc)(CPUState *cpu, vaddr value);
 int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
 int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index 460e7d63b0c..3f9a5199dd1 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -21,6 +21,10 @@ typedef struct SysemuCPUOps {
  */
 void (*get_memory_mapping)(CPUState *cpu, MemoryMappingList *list,
Error **errp);
+/**
+ * @get_paging_enabled: Callback for inquiring whether paging is enabled.
+ */
+bool (*get_paging_enabled)(const CPUState *cpu);
 /**
  * @get_phys_page_debug: Callback for obtaining a physical address.
  */
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 339bdfadd7a..7a8487d468f 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -71,8 +71,8 @@ bool cpu_paging_enabled(const CPUState *cpu)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-if (cc->get_paging_enabled) {
-return cc->get_paging_enabled(cpu);
+if (cc->sysemu_ops->get_paging_enabled) {
+return cc->sysemu_ops->get_paging_enabled(cpu);
 }
 
 return false;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d33ee9f831e..3519cef8fba 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7157,12 +7157,14 @@ static int64_t x86_cpu_get_arch_id(CPUState *cs)
 return cpu->apic_id;
 }
 
+#if !defined(CONFIG_USER_ONLY)
 static bool x86_cpu_get_paging_enabled(const CPUState *cs)
 {
 X86CPU *cpu = X86_CPU(cs);
 
 return cpu->env.cr[0] & CR0_PG_MASK;
 }
+#endif /* !CONFIG_USER_ONLY */
 
 static void x86_cpu_set_pc(CPUState *cs, vaddr value)
 {
@@ -7389,6 +7391,7 @@ static Property x86_cpu_properties[] = {
 #ifndef CONFIG_USER_ONLY
 static struct SysemuCPUOps i386_sysemu_ops = {
 .get_memory_mapping = x86_cpu_get_memory_mapping,
+.get_paging_enabled = x86_cpu_get_paging_enabled,
 .get_phys_page_attrs_debug = x86_cpu_get_phys_page_attrs_debug,
 .asidx_from_attrs = x86_asidx_from_attrs,
 .get_crash_info = x86_cpu_get_crash_info,
@@ -7429,7 +7432,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
void *data)
 cc->gdb_read_register = x86_cpu_gdb_read_register;
 cc->gdb_write_register = x86_cpu_gdb_write_register;
 cc->get_arch_id = x86_cpu_get_arch_id;
-cc->get_paging_enabled = x86_cpu_get_paging_enabled;
 
 #ifndef CONFIG_USER_ONLY
 cc->sysemu_ops = _sysemu_ops;
-- 
2.26.2

Re: [PATCH v2 17/42] esp: move pdma_len and TC logic into esp_pdma_read()/esp_pdma_write()

Le 09/02/2021 à 20:29, Mark Cave-Ayland a écrit :
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/scsi/esp.c | 50 --
>  1 file changed, 32 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index cfeba2feb0..7134c0aff4 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -153,22 +153,45 @@ static uint8_t *get_pdma_buf(ESPState *s)
>  
>  static uint8_t esp_pdma_read(ESPState *s)
>  {
> +uint32_t dmalen = esp_get_tc(s);
> +uint8_t val;
> +
> +if (dmalen == 0 || s->pdma_len == 0) {
> +return 0;
> +}
> +
>  switch (s->pdma_origin) {
>  case PDMA:
> -return s->pdma_buf[s->pdma_cur++];
> +val = s->pdma_buf[s->pdma_cur++];
> +break;
>  case TI:
> -return s->ti_buf[s->pdma_cur++];
> +val = s->ti_buf[s->pdma_cur++];
> +break;
>  case CMD:
> -return s->cmdbuf[s->pdma_cur++];
> +val = s->cmdbuf[s->pdma_cur++];
> +break;
>  case ASYNC:
> -return s->async_buf[s->pdma_cur++];
> +val = s->async_buf[s->pdma_cur++];
> +break;
>  default:
>  g_assert_not_reached();
>  }
> +
> +s->pdma_len--;
> +dmalen--;
> +esp_set_tc(s, dmalen);
> +
> +return val;
>  }
>  
>  static void esp_pdma_write(ESPState *s, uint8_t val)
>  {
> +uint32_t dmalen = esp_get_tc(s);
> +
> +if (dmalen == 0 || s->pdma_len == 0) {
> +return;
> +}
> +
>  switch (s->pdma_origin) {
>  case PDMA:
>  s->pdma_buf[s->pdma_cur++] = val;
> @@ -185,6 +208,10 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
>  default:
>  g_assert_not_reached();
>  }
> +
> +s->pdma_len--;
> +dmalen--;
> +esp_set_tc(s, dmalen);
>  }
>  
>  static int get_cmd_cb(ESPState *s)
> @@ -945,27 +972,18 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr 
> addr,
>  {
>  SysBusESPState *sysbus = opaque;
>  ESPState *s = ESP(>esp);
> -uint32_t dmalen = esp_get_tc(s);
>  
>  trace_esp_pdma_write(size);
>  
> -if (dmalen == 0 || s->pdma_len == 0) {
> -return;
> -}
>  switch (size) {
>  case 1:
>  esp_pdma_write(s, val);
> -s->pdma_len--;
> -dmalen--;
>  break;
>  case 2:
>  esp_pdma_write(s, val >> 8);
>  esp_pdma_write(s, val);
> -s->pdma_len -= 2;
> -dmalen -= 2;
>  break;
>  }
> -esp_set_tc(s, dmalen);
>  if (s->pdma_len == 0 && s->pdma_cb) {
>  esp_lower_drq(s);
>  s->pdma_cb(s);
> @@ -989,17 +1007,13 @@ static uint64_t sysbus_esp_pdma_read(void *opaque, 
> hwaddr addr,
>  switch (size) {
>  case 1:
>  val = esp_pdma_read(s);
> -s->pdma_len--;
> -dmalen--;
>  break;
>  case 2:
>  val = esp_pdma_read(s);
>  val = (val << 8) | esp_pdma_read(s);
> -s->pdma_len -= 2;
> -dmalen -= 2;
>  break;
>  }
> -esp_set_tc(s, dmalen);
> +dmalen = esp_get_tc(s);
>  if (dmalen == 0 || (s->pdma_len == 0 && s->pdma_cb)) {
>  esp_lower_drq(s);
>  s->pdma_cb(s);
> 

Reviewed-by: Laurent Vivier

Re: [PATCH v2 14/42] esp: remove minlen restriction in handle_ti

Le 09/02/2021 à 20:29, Mark Cave-Ayland a écrit :
> The limiting of DMA transfers to the maximum size of the available data is 
> already
> handled by esp_do_dma() and do_dma_pdma_cb().
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/scsi/esp.c | 12 ++--
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index fcc99f5fe4..e7cf36f4b8 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -553,7 +553,7 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
>  
>  static void handle_ti(ESPState *s)
>  {
> -uint32_t dmalen, minlen;
> +uint32_t dmalen;
>  
>  if (s->dma && !s->dma_enabled) {
>  s->dma_cb = handle_ti;
> @@ -561,16 +561,8 @@ static void handle_ti(ESPState *s)
>  }
>  
>  dmalen = esp_get_tc(s);
> -
> -if (s->do_cmd) {
> -minlen = (dmalen < ESP_CMDBUF_SZ) ? dmalen : ESP_CMDBUF_SZ;
> -} else if (s->ti_size < 0) {
> -minlen = (dmalen < -s->ti_size) ? dmalen : -s->ti_size;
> -} else {
> -minlen = (dmalen < s->ti_size) ? dmalen : s->ti_size;
> -}
> -trace_esp_handle_ti(minlen);
>  if (s->dma) {
> +trace_esp_handle_ti(dmalen);
>  s->rregs[ESP_RSTAT] &= ~STAT_TC;
>  esp_do_dma(s);
>  } else if (s->do_cmd) {
> 

Reviewed-by: Laurent Vivier

[PATCH v2 14/17] cpu: Move CPUClass::get_memory_mapping to SysemuCPUOps

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h| 3 ---
 include/hw/core/sysemu-cpu-ops.h | 5 +
 hw/core/cpu.c| 4 ++--
 target/i386/cpu.c| 2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 9a86c707cf7..8af78cdde23 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -94,7 +94,6 @@ struct AccelCPUClass;
  * @dump_statistics: Callback for dumping statistics.
  * @get_arch_id: Callback for getting architecture-dependent CPU ID.
  * @get_paging_enabled: Callback for inquiring whether paging is enabled.
- * @get_memory_mapping: Callback for obtaining the memory mappings.
  * @set_pc: Callback for setting the Program Counter register. This
  *   should have the semantics used by the target architecture when
  *   setting the PC from a source such as an ELF file entry point;
@@ -138,8 +137,6 @@ struct CPUClass {
 void (*dump_statistics)(CPUState *cpu, int flags);
 int64_t (*get_arch_id)(CPUState *cpu);
 bool (*get_paging_enabled)(const CPUState *cpu);
-void (*get_memory_mapping)(CPUState *cpu, MemoryMappingList *list,
-   Error **errp);
 void (*set_pc)(CPUState *cpu, vaddr value);
 int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
 int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index 0c8f616a565..460e7d63b0c 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -16,6 +16,11 @@
  * struct SysemuCPUOps: System operations specific to a CPU class
  */
 typedef struct SysemuCPUOps {
+/**
+ * @get_memory_mapping: Callback for obtaining the memory mappings.
+ */
+void (*get_memory_mapping)(CPUState *cpu, MemoryMappingList *list,
+   Error **errp);
 /**
  * @get_phys_page_debug: Callback for obtaining a physical address.
  */
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 6932781425a..339bdfadd7a 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -83,8 +83,8 @@ void cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList 
*list,
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-if (cc->get_memory_mapping) {
-cc->get_memory_mapping(cpu, list, errp);
+if (cc->sysemu_ops->get_memory_mapping) {
+cc->sysemu_ops->get_memory_mapping(cpu, list, errp);
 return;
 }
 
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index c7a18cd8e4f..d33ee9f831e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7388,6 +7388,7 @@ static Property x86_cpu_properties[] = {
 
 #ifndef CONFIG_USER_ONLY
 static struct SysemuCPUOps i386_sysemu_ops = {
+.get_memory_mapping = x86_cpu_get_memory_mapping,
 .get_phys_page_attrs_debug = x86_cpu_get_phys_page_attrs_debug,
 .asidx_from_attrs = x86_asidx_from_attrs,
 .get_crash_info = x86_cpu_get_crash_info,
@@ -7431,7 +7432,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
void *data)
 cc->get_paging_enabled = x86_cpu_get_paging_enabled;
 
 #ifndef CONFIG_USER_ONLY
-cc->get_memory_mapping = x86_cpu_get_memory_mapping;
 cc->sysemu_ops = _sysemu_ops;
 #endif /* !CONFIG_USER_ONLY */
 
-- 
2.26.2

[PATCH v2 13/17] cpu: Move CPUClass::get_phys_page_debug to SysemuCPUOps

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h|  8 
 include/hw/core/sysemu-cpu-ops.h | 13 +
 hw/core/cpu.c|  6 +++---
 target/alpha/cpu.c   |  2 +-
 target/arm/cpu.c |  2 +-
 target/avr/cpu.c |  2 +-
 target/cris/cpu.c|  2 +-
 target/hppa/cpu.c|  2 +-
 target/i386/cpu.c|  2 +-
 target/lm32/cpu.c|  2 +-
 target/m68k/cpu.c|  2 +-
 target/microblaze/cpu.c  |  2 +-
 target/mips/cpu.c|  2 +-
 target/moxie/cpu.c   |  4 +---
 target/nios2/cpu.c   |  2 +-
 target/openrisc/cpu.c|  2 +-
 target/riscv/cpu.c   |  2 +-
 target/rx/cpu.c  |  2 +-
 target/s390x/cpu.c   |  2 +-
 target/sh4/cpu.c |  2 +-
 target/sparc/cpu.c   |  2 +-
 target/tricore/cpu.c |  2 +-
 target/unicore32/cpu.c   |  2 +-
 target/xtensa/cpu.c  |  2 +-
 target/ppc/translate_init.c.inc  |  2 +-
 25 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 6713a615916..9a86c707cf7 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -103,11 +103,6 @@ struct AccelCPUClass;
  *   If the target behaviour here is anything other than "set
  *   the PC register to the value passed in" then the target must
  *   also implement the synchronize_from_tb hook.
- * @get_phys_page_debug: Callback for obtaining a physical address.
- * @get_phys_page_attrs_debug: Callback for obtaining a physical address and 
the
- *   associated memory transaction attributes to use for the access.
- *   CPUs which use memory transaction attributes should implement this
- *   instead of get_phys_page_debug.
  * @gdb_read_register: Callback for letting GDB read a register.
  * @gdb_write_register: Callback for letting GDB write a register.
  * @gdb_num_core_regs: Number of core registers accessible to GDB.
@@ -146,9 +141,6 @@ struct CPUClass {
 void (*get_memory_mapping)(CPUState *cpu, MemoryMappingList *list,
Error **errp);
 void (*set_pc)(CPUState *cpu, vaddr value);
-hwaddr (*get_phys_page_debug)(CPUState *cpu, vaddr addr);
-hwaddr (*get_phys_page_attrs_debug)(CPUState *cpu, vaddr addr,
-MemTxAttrs *attrs);
 int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
 int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
 
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index 3c3f211136d..0c8f616a565 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -16,6 +16,19 @@
  * struct SysemuCPUOps: System operations specific to a CPU class
  */
 typedef struct SysemuCPUOps {
+/**
+ * @get_phys_page_debug: Callback for obtaining a physical address.
+ */
+hwaddr (*get_phys_page_debug)(CPUState *cpu, vaddr addr);
+/**
+ * @get_phys_page_attrs_debug: Callback for obtaining a physical address
+ *   and the associated memory transaction attributes to use for the
+ *   access.
+ * CPUs which use memory transaction attributes should implement this
+ * instead of get_phys_page_debug.
+ */
+hwaddr (*get_phys_page_attrs_debug)(CPUState *cpu, vaddr addr,
+MemTxAttrs *attrs);
 /**
  * @asidx_from_attrs: Callback to return the CPU AddressSpace to use for
  *   a memory access with the specified memory transaction attributes.
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index c44229205ff..6932781425a 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -96,12 +96,12 @@ hwaddr cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr 
addr,
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-if (cc->get_phys_page_attrs_debug) {
-return cc->get_phys_page_attrs_debug(cpu, addr, attrs);
+if (cc->sysemu_ops->get_phys_page_attrs_debug) {
+return cc->sysemu_ops->get_phys_page_attrs_debug(cpu, addr, attrs);
 }
 /* Fallback for CPUs which don't implement the _attrs_ hook */
 *attrs = MEMTXATTRS_UNSPECIFIED;
-return cc->get_phys_page_debug(cpu, addr);
+return cc->sysemu_ops->get_phys_page_debug(cpu, addr);
 }
 
 hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr)
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index 8d7a73d638e..d9a51d9f647 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -208,6 +208,7 @@ static void alpha_cpu_initfn(Object *obj)
 
 #ifndef CONFIG_USER_ONLY
 static struct SysemuCPUOps alpha_sysemu_ops = {
+.get_phys_page_debug = alpha_cpu_get_phys_page_debug,
 .vmsd = _alpha_cpu,
 };
 #endif
@@ -242,7 +243,6 @@ static void alpha_cpu_class_init(ObjectClass *oc, void 
*data)
 cc->gdb_read_register =

[PATCH v2 12/17] cpu: Move CPUClass::asidx_from_attrs to SysemuCPUOps

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h| 3 ---
 include/hw/core/sysemu-cpu-ops.h | 5 +
 hw/core/cpu.c| 4 ++--
 target/arm/cpu.c | 2 +-
 target/i386/cpu.c| 2 +-
 5 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 0a2c29c3735..6713a615916 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -108,8 +108,6 @@ struct AccelCPUClass;
  *   associated memory transaction attributes to use for the access.
  *   CPUs which use memory transaction attributes should implement this
  *   instead of get_phys_page_debug.
- * @asidx_from_attrs: Callback to return the CPU AddressSpace to use for
- *   a memory access with the specified memory transaction attributes.
  * @gdb_read_register: Callback for letting GDB read a register.
  * @gdb_write_register: Callback for letting GDB write a register.
  * @gdb_num_core_regs: Number of core registers accessible to GDB.
@@ -151,7 +149,6 @@ struct CPUClass {
 hwaddr (*get_phys_page_debug)(CPUState *cpu, vaddr addr);
 hwaddr (*get_phys_page_attrs_debug)(CPUState *cpu, vaddr addr,
 MemTxAttrs *attrs);
-int (*asidx_from_attrs)(CPUState *cpu, MemTxAttrs attrs);
 int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
 int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
 
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index 60c667801ef..3c3f211136d 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -16,6 +16,11 @@
  * struct SysemuCPUOps: System operations specific to a CPU class
  */
 typedef struct SysemuCPUOps {
+/**
+ * @asidx_from_attrs: Callback to return the CPU AddressSpace to use for
+ *   a memory access with the specified memory transaction attributes.
+ */
+int (*asidx_from_attrs)(CPUState *cpu, MemTxAttrs attrs);
 /**
  * @get_crash_info: Callback for reporting guest crash information in
  * GUEST_PANICKED events.
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index c74390aafbf..c44229205ff 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -116,8 +116,8 @@ int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs)
 CPUClass *cc = CPU_GET_CLASS(cpu);
 int ret = 0;
 
-if (cc->asidx_from_attrs) {
-ret = cc->asidx_from_attrs(cpu, attrs);
+if (cc->sysemu_ops->asidx_from_attrs) {
+ret = cc->sysemu_ops->asidx_from_attrs(cpu, attrs);
 assert(ret < cpu->num_ases && ret >= 0);
 }
 return ret;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 7dc6956f2cc..acaa3ab68da 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2262,6 +2262,7 @@ static gchar *arm_gdb_arch_name(CPUState *cs)
 
 #ifndef CONFIG_USER_ONLY
 static struct SysemuCPUOps arm_sysemu_ops = {
+.asidx_from_attrs = arm_asidx_from_attrs,
 .write_elf32_note = arm_cpu_write_elf32_note,
 .write_elf64_note = arm_cpu_write_elf64_note,
 .virtio_is_big_endian = arm_cpu_virtio_is_big_endian,
@@ -2307,7 +2308,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void 
*data)
 cc->gdb_write_register = arm_cpu_gdb_write_register;
 #ifndef CONFIG_USER_ONLY
 cc->get_phys_page_attrs_debug = arm_cpu_get_phys_page_attrs_debug;
-cc->asidx_from_attrs = arm_asidx_from_attrs;
 cc->sysemu_ops = _sysemu_ops;
 #endif
 cc->gdb_num_core_regs = 26;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b26905b22a3..10884540610 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7388,6 +7388,7 @@ static Property x86_cpu_properties[] = {
 
 #ifndef CONFIG_USER_ONLY
 static struct SysemuCPUOps i386_sysemu_ops = {
+.asidx_from_attrs = x86_asidx_from_attrs,
 .get_crash_info = x86_cpu_get_crash_info,
 .write_elf32_note = x86_cpu_write_elf32_note,
 .write_elf64_note = x86_cpu_write_elf64_note,
@@ -7429,7 +7430,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
void *data)
 cc->get_paging_enabled = x86_cpu_get_paging_enabled;
 
 #ifndef CONFIG_USER_ONLY
-cc->asidx_from_attrs = x86_asidx_from_attrs;
 cc->get_memory_mapping = x86_cpu_get_memory_mapping;
 cc->get_phys_page_attrs_debug = x86_cpu_get_phys_page_attrs_debug;
 cc->sysemu_ops = _sysemu_ops;
-- 
2.26.2

[RFC PATCH v2 17/17] cpu: Restrict "hw/core/sysemu-cpu-ops.h" to target/cpu.c

Somehow similar to commit 78271684719 ("cpu: tcg_ops: move to
tcg-cpu-ops.h, keep a pointer in CPUClass"):

We cannot in principle make the SysEmu Operations field definitions
conditional on CONFIG_SOFTMMU in code that is included by both
common_ss and specific_ss modules.

Therefore, what we can do safely to restrict the SysEmu fields to
system emulation builds, is to move all sysemu operations into a
separate header file, which is only included by system-specific code.

This leaves just a NULL pointer in the cpu.h for the user-mode builds.

Inspired-by: Claudio Fontana 
Signed-off-by: Philippe Mathieu-Daudé 
---
RFC: improve commit description?

 include/hw/core/cpu.h   | 3 ++-
 cpu.c   | 1 +
 hw/core/cpu.c   | 1 +
 target/alpha/cpu.c  | 1 +
 target/arm/cpu.c| 1 +
 target/avr/cpu.c| 1 +
 target/cris/cpu.c   | 1 +
 target/hppa/cpu.c   | 1 +
 target/i386/cpu.c   | 1 +
 target/m68k/cpu.c   | 1 +
 target/microblaze/cpu.c | 1 +
 target/mips/cpu.c   | 1 +
 target/moxie/cpu.c  | 1 +
 target/nios2/cpu.c  | 1 +
 target/openrisc/cpu.c   | 1 +
 target/riscv/cpu.c  | 1 +
 target/rx/cpu.c | 1 +
 target/s390x/cpu.c  | 1 +
 target/sh4/cpu.c| 1 +
 target/sparc/cpu.c  | 1 +
 target/tricore/cpu.c| 1 +
 target/xtensa/cpu.c | 1 +
 target/ppc/translate_init.c.inc | 1 +
 23 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index d99d3c830dc..398696f0f2d 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -80,7 +80,8 @@ struct TCGCPUOps;
 /* see accel-cpu.h */
 struct AccelCPUClass;
 
-#include "hw/core/sysemu-cpu-ops.h"
+/* see sysemu-cpu-ops.h */
+struct SysemuCPUOps;
 
 /**
  * CPUClass:
diff --git a/cpu.c b/cpu.c
index 64e17537e21..29dafee581f 100644
--- a/cpu.c
+++ b/cpu.c
@@ -29,6 +29,7 @@
 #ifdef CONFIG_USER_ONLY
 #include "qemu.h"
 #else
+#include "hw/core/sysemu-cpu-ops.h"
 #include "exec/address-spaces.h"
 #endif
 #include "sysemu/tcg.h"
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 7a8487d468f..da7543be514 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -35,6 +35,7 @@
 #include "trace/trace-root.h"
 #include "qemu/plugin.h"
 #include "sysemu/hw_accel.h"
+#include "hw/core/sysemu-cpu-ops.h"
 
 CPUState *cpu_by_arch_id(int64_t id)
 {
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index d9a51d9f647..f6b4bb14cc5 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -24,6 +24,7 @@
 #include "qemu/qemu-print.h"
 #include "cpu.h"
 #include "exec/exec-all.h"
+#include "hw/core/sysemu-cpu-ops.h"
 
 
 static void alpha_cpu_set_pc(CPUState *cs, vaddr value)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 6cd546213de..7fa22a6beba 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -35,6 +35,7 @@
 #if !defined(CONFIG_USER_ONLY)
 #include "hw/loader.h"
 #include "hw/boards.h"
+#include "hw/core/sysemu-cpu-ops.h"
 #endif
 #include "sysemu/sysemu.h"
 #include "sysemu/tcg.h"
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 040d3526995..89de301fc2b 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -24,6 +24,7 @@
 #include "exec/exec-all.h"
 #include "cpu.h"
 #include "disas/dis-asm.h"
+#include "hw/core/sysemu-cpu-ops.h"
 
 static void avr_cpu_set_pc(CPUState *cs, vaddr value)
 {
diff --git a/target/cris/cpu.c b/target/cris/cpu.c
index 77f821f4d9a..ed944094cf3 100644
--- a/target/cris/cpu.c
+++ b/target/cris/cpu.c
@@ -26,6 +26,7 @@
 #include "qemu/qemu-print.h"
 #include "cpu.h"
 #include "mmu.h"
+#include "hw/core/sysemu-cpu-ops.h"
 
 
 static void cris_cpu_set_pc(CPUState *cs, vaddr value)
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index 7de37aadd4d..304a975eddf 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -25,6 +25,7 @@
 #include "qemu/module.h"
 #include "exec/exec-all.h"
 #include "fpu/softfloat.h"
+#include "hw/core/sysemu-cpu-ops.h"
 
 
 static void hppa_cpu_set_pc(CPUState *cs, vaddr value)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3519cef8fba..1e8ee015bfc 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -60,6 +60,7 @@
 #include "exec/address-spaces.h"
 #include "hw/i386/apic_internal.h"
 #include "hw/boards.h"
+#include "hw/core/sysemu-cpu-ops.h"
 #endif
 
 #include "disas/capstone.h"
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index eaf5f34d22c..96fe37e84f1 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -23,6 +23,7 @@
 #include "cpu.h"
 #include "migration/vmstate.h"
 #include "fpu/softfloat.h"
+#include "hw/core/sysemu-cpu-ops.h"
 
 static void m68k_cpu_set_pc(CPUState *cs, vaddr value)
 {
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index a21f15192ae..ad3996cd90e 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -28,6 +28,7 @@

[PATCH v2 10/17] cpu: Move CPUClass::get_crash_info to SysemuCPUOps

cpu_get_crash_info() is called on GUEST_PANICKED events,
which only occur in system emulation.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h| 1 -
 include/hw/core/sysemu-cpu-ops.h | 5 +
 hw/core/cpu.c| 4 ++--
 target/i386/cpu.c| 2 +-
 target/s390x/cpu.c   | 2 +-
 5 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index dfb50b60128..781cd8fc42b 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -150,7 +150,6 @@ struct CPUClass {
 int (*memory_rw_debug)(CPUState *cpu, vaddr addr,
uint8_t *buf, int len, bool is_write);
 void (*dump_state)(CPUState *cpu, FILE *, int flags);
-GuestPanicInformation* (*get_crash_info)(CPUState *cpu);
 void (*dump_statistics)(CPUState *cpu, int flags);
 int64_t (*get_arch_id)(CPUState *cpu);
 bool (*get_paging_enabled)(const CPUState *cpu);
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index 9c3ac4f2280..b9ffca07665 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -16,6 +16,11 @@
  * struct SysemuCPUOps: System operations specific to a CPU class
  */
 typedef struct SysemuCPUOps {
+/**
+ * @get_crash_info: Callback for reporting guest crash information in
+ * GUEST_PANICKED events.
+ */
+GuestPanicInformation* (*get_crash_info)(CPUState *cpu);
 /**
  * @virtio_is_big_endian: Callback to return %true if a CPU which supports
  *   runtime configurable endianness is currently big-endian.
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 09eaa3fa49f..0aebc18c41f 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -220,8 +220,8 @@ GuestPanicInformation *cpu_get_crash_info(CPUState *cpu)
 CPUClass *cc = CPU_GET_CLASS(cpu);
 GuestPanicInformation *res = NULL;
 
-if (cc->get_crash_info) {
-res = cc->get_crash_info(cpu);
+if (cc->sysemu_ops->get_crash_info) {
+res = cc->sysemu_ops->get_crash_info(cpu);
 }
 return res;
 }
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 2d1e61da8ea..b7672a7accc 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7388,6 +7388,7 @@ static Property x86_cpu_properties[] = {
 
 #ifndef CONFIG_USER_ONLY
 static struct SysemuCPUOps i386_sysemu_ops = {
+.get_crash_info = x86_cpu_get_crash_info,
 .vmsd = _x86_cpu,
 };
 #endif
@@ -7427,7 +7428,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
void *data)
 cc->asidx_from_attrs = x86_asidx_from_attrs;
 cc->get_memory_mapping = x86_cpu_get_memory_mapping;
 cc->get_phys_page_attrs_debug = x86_cpu_get_phys_page_attrs_debug;
-cc->get_crash_info = x86_cpu_get_crash_info;
 cc->write_elf64_note = x86_cpu_write_elf64_note;
 cc->write_elf64_qemunote = x86_cpu_write_elf64_qemunote;
 cc->write_elf32_note = x86_cpu_write_elf32_note;
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index a480f4abbaf..04c14fcd9da 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -479,6 +479,7 @@ static void s390_cpu_reset_full(DeviceState *dev)
 
 #ifndef CONFIG_USER_ONLY
 static struct SysemuCPUOps s390_sysemu_ops = {
+.get_crash_info = s390_cpu_get_crash_info,
 .vmsd = _s390_cpu,
 };
 #endif
@@ -523,7 +524,6 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
 cc->gdb_write_register = s390_cpu_gdb_write_register;
 #ifndef CONFIG_USER_ONLY
 cc->get_phys_page_debug = s390_cpu_get_phys_page_debug;
-cc->get_crash_info = s390_cpu_get_crash_info;
 cc->write_elf64_note = s390_cpu_write_elf64_note;
 cc->sysemu_ops = _sysemu_ops;
 #endif
-- 
2.26.2

[PATCH v2 11/17] cpu: Move CPUClass::write_elf* to SysemuCPUOps

The write_elf*() handlers are used to dump vmcore images.
This feature is only meaningful for system emulation.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h| 17 -
 include/hw/core/sysemu-cpu-ops.h | 24 
 hw/core/cpu.c| 16 
 target/arm/cpu.c |  4 ++--
 target/i386/cpu.c|  8 
 target/s390x/cpu.c   |  2 +-
 target/ppc/translate_init.c.inc  |  6 ++
 7 files changed, 41 insertions(+), 36 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 781cd8fc42b..0a2c29c3735 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -112,14 +112,6 @@ struct AccelCPUClass;
  *   a memory access with the specified memory transaction attributes.
  * @gdb_read_register: Callback for letting GDB read a register.
  * @gdb_write_register: Callback for letting GDB write a register.
- * @write_elf64_note: Callback for writing a CPU-specific ELF note to a
- * 64-bit VM coredump.
- * @write_elf32_qemunote: Callback for writing a CPU- and QEMU-specific ELF
- * note to a 32-bit VM coredump.
- * @write_elf32_note: Callback for writing a CPU-specific ELF note to a
- * 32-bit VM coredump.
- * @write_elf32_qemunote: Callback for writing a CPU- and QEMU-specific ELF
- * note to a 32-bit VM coredump.
  * @gdb_num_core_regs: Number of core registers accessible to GDB.
  * @gdb_core_xml_file: File name for core registers GDB XML description.
  * @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop
@@ -163,15 +155,6 @@ struct CPUClass {
 int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
 int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
 
-int (*write_elf64_note)(WriteCoreDumpFunction f, CPUState *cpu,
-int cpuid, void *opaque);
-int (*write_elf64_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
-void *opaque);
-int (*write_elf32_note)(WriteCoreDumpFunction f, CPUState *cpu,
-int cpuid, void *opaque);
-int (*write_elf32_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
-void *opaque);
-
 const char *gdb_core_xml_file;
 gchar * (*gdb_arch_name)(CPUState *cpu);
 const char * (*gdb_get_dynamic_xml)(CPUState *cpu, const char *xmlname);
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index b9ffca07665..60c667801ef 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -21,6 +21,30 @@ typedef struct SysemuCPUOps {
  * GUEST_PANICKED events.
  */
 GuestPanicInformation* (*get_crash_info)(CPUState *cpu);
+/**
+ * @write_elf32_note: Callback for writing a CPU-specific ELF note to a
+ * 32-bit VM coredump.
+ */
+int (*write_elf32_note)(WriteCoreDumpFunction f, CPUState *cpu,
+int cpuid, void *opaque);
+/**
+ * @write_elf64_note: Callback for writing a CPU-specific ELF note to a
+ * 64-bit VM coredump.
+ */
+int (*write_elf64_note)(WriteCoreDumpFunction f, CPUState *cpu,
+int cpuid, void *opaque);
+/**
+ * @write_elf32_qemunote: Callback for writing a CPU- and QEMU-specific ELF
+ * note to a 32-bit VM coredump.
+ */
+int (*write_elf32_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
+void *opaque);
+/**
+ * @write_elf64_qemunote: Callback for writing a CPU- and QEMU-specific ELF
+ * note to a 64-bit VM coredump.
+ */
+int (*write_elf64_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
+void *opaque);
 /**
  * @virtio_is_big_endian: Callback to return %true if a CPU which supports
  *   runtime configurable endianness is currently big-endian.
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 0aebc18c41f..c74390aafbf 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -151,10 +151,10 @@ int cpu_write_elf32_qemunote(WriteCoreDumpFunction f, 
CPUState *cpu,
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-if (!cc->write_elf32_qemunote) {
+if (!cc->sysemu_ops->write_elf32_qemunote) {
 return 0;
 }
-return (*cc->write_elf32_qemunote)(f, cpu, opaque);
+return (*cc->sysemu_ops->write_elf32_qemunote)(f, cpu, opaque);
 }
 
 int cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cpu,
@@ -162,10 +162,10 @@ int cpu_write_elf32_note(WriteCoreDumpFunction f, 
CPUState *cpu,
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-if (!cc->write_elf32_note) {
+if (!cc->sysemu_ops->write_elf32_note) {
 return -1;
 }
-return (*cc->write_elf32_note)(f, cpu, cpuid, opaque);
+return (*cc->sysemu_ops->write_elf32_note)(f, cpu, cpuid, opaque);
 }
 
 int cpu_write_elf64_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
@@ -173,10 +173,10

[PATCH v2 08/17] cpu: Move CPUClass::vmsd to SysemuCPUOps

Migration is specific to system emulation.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h|  2 --
 include/hw/core/sysemu-cpu-ops.h |  4 
 cpu.c| 18 --
 target/alpha/cpu.c   |  2 +-
 target/arm/cpu.c |  2 +-
 target/avr/cpu.c |  2 +-
 target/cris/cpu.c|  2 +-
 target/hppa/cpu.c|  2 +-
 target/i386/cpu.c|  2 +-
 target/lm32/cpu.c|  2 +-
 target/m68k/cpu.c|  2 +-
 target/microblaze/cpu.c  |  2 +-
 target/mips/cpu.c|  2 +-
 target/moxie/cpu.c   |  2 +-
 target/nios2/cpu.c   |  7 +++
 target/openrisc/cpu.c|  2 +-
 target/riscv/cpu.c   |  4 ++--
 target/rx/cpu.c  |  6 ++
 target/s390x/cpu.c   |  2 +-
 target/sh4/cpu.c |  4 ++--
 target/sparc/cpu.c   |  2 +-
 target/tricore/cpu.c |  7 +++
 target/unicore32/cpu.c   |  2 +-
 target/xtensa/cpu.c  |  4 ++--
 target/ppc/translate_init.c.inc  |  2 +-
 25 files changed, 54 insertions(+), 34 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 3c26471d0fa..471c99d9f04 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -124,7 +124,6 @@ struct AccelCPUClass;
  * 32-bit VM coredump.
  * @write_elf32_qemunote: Callback for writing a CPU- and QEMU-specific ELF
  * note to a 32-bit VM coredump.
- * @vmsd: State description for migration.
  * @gdb_num_core_regs: Number of core registers accessible to GDB.
  * @gdb_core_xml_file: File name for core registers GDB XML description.
  * @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop
@@ -179,7 +178,6 @@ struct CPUClass {
 int (*write_elf32_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
 void *opaque);
 
-const VMStateDescription *vmsd;
 const char *gdb_core_xml_file;
 gchar * (*gdb_arch_name)(CPUState *cpu);
 const char * (*gdb_get_dynamic_xml)(CPUState *cpu, const char *xmlname);
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index e54a08ea25e..05f19b22070 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -16,6 +16,10 @@
  * struct SysemuCPUOps: System operations specific to a CPU class
  */
 typedef struct SysemuCPUOps {
+/**
+ * @vmsd: State description for migration.
+ */
+const VMStateDescription *vmsd;
 } SysemuCPUOps;
 
 #endif /* SYSEMU_CPU_OPS_H */
diff --git a/cpu.c b/cpu.c
index bfbe5a66f95..64e17537e21 100644
--- a/cpu.c
+++ b/cpu.c
@@ -126,7 +126,9 @@ const VMStateDescription vmstate_cpu_common = {
 
 void cpu_exec_realizefn(CPUState *cpu, Error **errp)
 {
+#ifndef CONFIG_USER_ONLY
 CPUClass *cc = CPU_GET_CLASS(cpu);
+#endif
 
 cpu_list_add(cpu);
 
@@ -137,27 +139,23 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
 }
 #endif /* CONFIG_TCG */
 
-#ifdef CONFIG_USER_ONLY
-assert(cc->vmsd == NULL);
-#else
+#ifndef CONFIG_USER_ONLY
 if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
 vmstate_register(NULL, cpu->cpu_index, _cpu_common, cpu);
 }
-if (cc->vmsd != NULL) {
-vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu);
+if (cc->sysemu_ops->vmsd != NULL) {
+vmstate_register(NULL, cpu->cpu_index, cc->sysemu_ops->vmsd, cpu);
 }
 #endif /* CONFIG_USER_ONLY */
 }
 
 void cpu_exec_unrealizefn(CPUState *cpu)
 {
+#ifndef CONFIG_USER_ONLY
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-#ifdef CONFIG_USER_ONLY
-assert(cc->vmsd == NULL);
-#else
-if (cc->vmsd != NULL) {
-vmstate_unregister(NULL, cc->vmsd, cpu);
+if (cc->sysemu_ops->vmsd != NULL) {
+vmstate_unregister(NULL, cc->sysemu_ops->vmsd, cpu);
 }
 if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
 vmstate_unregister(NULL, _cpu_common, cpu);
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index b9b431102f2..8d7a73d638e 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -208,6 +208,7 @@ static void alpha_cpu_initfn(Object *obj)
 
 #ifndef CONFIG_USER_ONLY
 static struct SysemuCPUOps alpha_sysemu_ops = {
+.vmsd = _alpha_cpu,
 };
 #endif
 
@@ -242,7 +243,6 @@ static void alpha_cpu_class_init(ObjectClass *oc, void 
*data)
 cc->gdb_write_register = alpha_cpu_gdb_write_register;
 #ifndef CONFIG_USER_ONLY
 cc->get_phys_page_debug = alpha_cpu_get_phys_page_debug;
-cc->vmsd = _alpha_cpu;
 cc->sysemu_ops = _sysemu_ops;
 #endif
 cc->disas_set_info = alpha_cpu_disas_set_info;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 994e7b344d4..e03977e4c3c 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2262,6 +2262,7 @@ static gchar *arm_gdb_arch_name(CPUState *cs)
 
 #ifndef CONFIG_USER_ONLY
 static struct SysemuCPUOps arm_sysemu_ops = {
+.vmsd = _arm_cpu,
 };
 #endif
 
@@

[PATCH v2 09/17] cpu: Move CPUClass::virtio_is_big_endian to SysemuCPUOps

VirtIO devices are only meaningful with system emulation.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h| 5 -
 include/hw/core/sysemu-cpu-ops.h | 8 
 hw/core/cpu.c| 4 ++--
 target/arm/cpu.c | 2 +-
 target/ppc/translate_init.c.inc  | 4 +---
 5 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 471c99d9f04..dfb50b60128 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -89,10 +89,6 @@ struct AccelCPUClass;
  * @parse_features: Callback to parse command line arguments.
  * @reset_dump_flags: #CPUDumpFlags to use for reset logging.
  * @has_work: Callback for checking if there is work to do.
- * @virtio_is_big_endian: Callback to return %true if a CPU which supports
- * runtime configurable endianness is currently big-endian. Non-configurable
- * CPUs can use the default implementation of this method. This method should
- * not be used by any callers other than the pre-1.0 virtio devices.
  * @memory_rw_debug: Callback for GDB memory access.
  * @dump_state: Callback for dumping state.
  * @dump_statistics: Callback for dumping statistics.
@@ -151,7 +147,6 @@ struct CPUClass {
 
 int reset_dump_flags;
 bool (*has_work)(CPUState *cpu);
-bool (*virtio_is_big_endian)(CPUState *cpu);
 int (*memory_rw_debug)(CPUState *cpu, vaddr addr,
uint8_t *buf, int len, bool is_write);
 void (*dump_state)(CPUState *cpu, FILE *, int flags);
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index 05f19b22070..9c3ac4f2280 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -16,6 +16,14 @@
  * struct SysemuCPUOps: System operations specific to a CPU class
  */
 typedef struct SysemuCPUOps {
+/**
+ * @virtio_is_big_endian: Callback to return %true if a CPU which supports
+ *   runtime configurable endianness is currently big-endian.
+ * Non-configurable CPUs can use the default implementation of this method.
+ * This method should not be used by any callers other than the pre-1.0
+ * virtio devices.
+ */
+bool (*virtio_is_big_endian)(CPUState *cpu);
 /**
  * @vmsd: State description for migration.
  */
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 5abf8bed2e4..09eaa3fa49f 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -204,8 +204,8 @@ bool cpu_virtio_is_big_endian(CPUState *cpu)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-if (cc->virtio_is_big_endian) {
-return cc->virtio_is_big_endian(cpu);
+if (cc->sysemu_ops->virtio_is_big_endian) {
+return cc->sysemu_ops->virtio_is_big_endian(cpu);
 }
 return target_words_bigendian();
 }
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index e03977e4c3c..2bad6307cce 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2262,6 +2262,7 @@ static gchar *arm_gdb_arch_name(CPUState *cs)
 
 #ifndef CONFIG_USER_ONLY
 static struct SysemuCPUOps arm_sysemu_ops = {
+.virtio_is_big_endian = arm_cpu_virtio_is_big_endian,
 .vmsd = _arm_cpu,
 };
 #endif
@@ -2305,7 +2306,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void 
*data)
 #ifndef CONFIG_USER_ONLY
 cc->get_phys_page_attrs_debug = arm_cpu_get_phys_page_attrs_debug;
 cc->asidx_from_attrs = arm_asidx_from_attrs;
-cc->virtio_is_big_endian = arm_cpu_virtio_is_big_endian;
 cc->write_elf64_note = arm_cpu_write_elf64_note;
 cc->write_elf32_note = arm_cpu_write_elf32_note;
 cc->sysemu_ops = _sysemu_ops;
diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index b5ed1dbfd26..2dd4f47adbb 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -10845,6 +10845,7 @@ static Property ppc_cpu_properties[] = {
 
 #ifndef CONFIG_USER_ONLY
 static struct SysemuCPUOps ppc_sysemu_ops = {
+.virtio_is_big_endian = ppc_cpu_is_big_endian,
 .vmsd = _ppc_cpu,
 };
 #endif
@@ -10913,9 +10914,6 @@ static void ppc_cpu_class_init(ObjectClass *oc, void 
*data)
 cc->gdb_core_xml_file = "power64-core.xml";
 #else
 cc->gdb_core_xml_file = "power-core.xml";
-#endif
-#ifndef CONFIG_USER_ONLY
-cc->virtio_is_big_endian = ppc_cpu_is_big_endian;
 #endif
 cc->disas_set_info = ppc_disas_set_info;
 
-- 
2.26.2

[PATCH v2 07/17] cpu: Introduce SysemuCPUOps structure

Introduce a structure to hold handler specific to sysemu.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h|  5 +
 include/hw/core/sysemu-cpu-ops.h | 21 +
 target/alpha/cpu.c   |  6 ++
 target/arm/cpu.c |  6 ++
 target/avr/cpu.c |  4 
 target/cris/cpu.c|  6 ++
 target/hppa/cpu.c|  6 ++
 target/i386/cpu.c|  6 ++
 target/lm32/cpu.c|  6 ++
 target/m68k/cpu.c|  6 ++
 target/microblaze/cpu.c  |  6 ++
 target/mips/cpu.c|  6 ++
 target/moxie/cpu.c   |  4 
 target/nios2/cpu.c   |  6 ++
 target/openrisc/cpu.c|  6 ++
 target/riscv/cpu.c   |  6 ++
 target/rx/cpu.c  |  8 
 target/s390x/cpu.c   |  6 ++
 target/sh4/cpu.c |  6 ++
 target/sparc/cpu.c   |  6 ++
 target/tricore/cpu.c |  4 
 target/unicore32/cpu.c   |  4 
 target/xtensa/cpu.c  |  6 ++
 target/ppc/translate_init.c.inc  |  6 ++
 24 files changed, 152 insertions(+)
 create mode 100644 include/hw/core/sysemu-cpu-ops.h

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index b12028c3c03..3c26471d0fa 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -80,6 +80,8 @@ struct TCGCPUOps;
 /* see accel-cpu.h */
 struct AccelCPUClass;
 
+#include "hw/core/sysemu-cpu-ops.h"
+
 /**
  * CPUClass:
  * @class_by_name: Callback to map -cpu command line model name to an
@@ -190,6 +192,9 @@ struct CPUClass {
 bool gdb_stop_before_watchpoint;
 struct AccelCPUClass *accel_cpu;
 
+/* when system emulation is not available, this pointer is NULL */
+const struct SysemuCPUOps *sysemu_ops;
+
 /* when TCG is not available, this pointer is NULL */
 struct TCGCPUOps *tcg_ops;
 };
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
new file mode 100644
index 000..e54a08ea25e
--- /dev/null
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -0,0 +1,21 @@
+/*
+ * CPU operations specific to system emulation
+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef SYSEMU_CPU_OPS_H
+#define SYSEMU_CPU_OPS_H
+
+#include "hw/core/cpu.h"
+
+/*
+ * struct SysemuCPUOps: System operations specific to a CPU class
+ */
+typedef struct SysemuCPUOps {
+} SysemuCPUOps;
+
+#endif /* SYSEMU_CPU_OPS_H */
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index faabffe0796..b9b431102f2 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -206,6 +206,11 @@ static void alpha_cpu_initfn(Object *obj)
 #endif
 }
 
+#ifndef CONFIG_USER_ONLY
+static struct SysemuCPUOps alpha_sysemu_ops = {
+};
+#endif
+
 #include "hw/core/tcg-cpu-ops.h"
 
 static struct TCGCPUOps alpha_tcg_ops = {
@@ -238,6 +243,7 @@ static void alpha_cpu_class_init(ObjectClass *oc, void 
*data)
 #ifndef CONFIG_USER_ONLY
 cc->get_phys_page_debug = alpha_cpu_get_phys_page_debug;
 cc->vmsd = _alpha_cpu;
+cc->sysemu_ops = _sysemu_ops;
 #endif
 cc->disas_set_info = alpha_cpu_disas_set_info;
 
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index b8bc89e71fc..994e7b344d4 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2260,6 +2260,11 @@ static gchar *arm_gdb_arch_name(CPUState *cs)
 return g_strdup("arm");
 }
 
+#ifndef CONFIG_USER_ONLY
+static struct SysemuCPUOps arm_sysemu_ops = {
+};
+#endif
+
 #ifdef CONFIG_TCG
 static struct TCGCPUOps arm_tcg_ops = {
 .initialize = arm_translate_init,
@@ -2303,6 +2308,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void 
*data)
 cc->virtio_is_big_endian = arm_cpu_virtio_is_big_endian;
 cc->write_elf64_note = arm_cpu_write_elf64_note;
 cc->write_elf32_note = arm_cpu_write_elf32_note;
+cc->sysemu_ops = _sysemu_ops;
 #endif
 cc->gdb_num_core_regs = 26;
 cc->gdb_core_xml_file = "arm-core.xml";
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 0f4596932ba..84f7ad4167e 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -184,6 +184,9 @@ static void avr_cpu_dump_state(CPUState *cs, FILE *f, int 
flags)
 qemu_fprintf(f, "\n");
 }
 
+static struct SysemuCPUOps avr_sysemu_ops = {
+};
+
 #include "hw/core/tcg-cpu-ops.h"
 
 static struct TCGCPUOps avr_tcg_ops = {
@@ -214,6 +217,7 @@ static void avr_cpu_class_init(ObjectClass *oc, void *data)
 cc->memory_rw_debug = avr_cpu_memory_rw_debug;
 cc->get_phys_page_debug = avr_cpu_get_phys_page_debug;
 cc->vmsd = _avr_cpu;
+cc->sysemu_ops = _sysemu_ops;
 cc->disas_set_info = avr_cpu_disas_set_info;
 cc->gdb_read_register = avr_cpu_gdb_read_register;
 cc->gdb_write_register = avr_cpu_gdb_write_register;
diff --git a/target/cris/cpu.c

[PATCH v2 05/17] cpu: Directly use get_paging_enabled() fallback handlers in place

No code uses CPUClass::get_paging_enabled() outside of hw/core/cpu.c:

  $ git grep -F -- '->get_paging_enabled'
  hw/core/cpu.c:74:return cc->get_paging_enabled(cpu);
  hw/core/cpu.c:438:k->get_paging_enabled = cpu_common_get_paging_enabled;
  target/i386/cpu.c:7418:cc->get_paging_enabled = 
x86_cpu_get_paging_enabled;

Check the handler presence in place and remove the common fallback code.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/core/cpu.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index a9ee2c74ec5..1de00bbb474 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -71,11 +71,10 @@ bool cpu_paging_enabled(const CPUState *cpu)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-return cc->get_paging_enabled(cpu);
-}
+if (cc->get_paging_enabled) {
+return cc->get_paging_enabled(cpu);
+}
 
-static bool cpu_common_get_paging_enabled(const CPUState *cpu)
-{
 return false;
 }
 
@@ -420,7 +419,6 @@ static void cpu_class_init(ObjectClass *klass, void *data)
 k->parse_features = cpu_common_parse_features;
 k->get_arch_id = cpu_common_get_arch_id;
 k->has_work = cpu_common_has_work;
-k->get_paging_enabled = cpu_common_get_paging_enabled;
 k->get_memory_mapping = cpu_common_get_memory_mapping;
 k->gdb_read_register = cpu_common_gdb_read_register;
 k->gdb_write_register = cpu_common_gdb_write_register;
-- 
2.26.2

[PATCH v2 06/17] cpu: Directly use get_memory_mapping() fallback handlers in place

No code uses CPUClass::get_memory_mapping() outside of hw/core/cpu.c:

  $ git grep -F -- '->get_memory_mapping'
  hw/core/cpu.c:87:cc->get_memory_mapping(cpu, list, errp);
  hw/core/cpu.c:439:k->get_memory_mapping = cpu_common_get_memory_mapping;
  target/i386/cpu.c:7422:cc->get_memory_mapping = 
x86_cpu_get_memory_mapping;

Check the handler presence in place and remove the common fallback code.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/core/cpu.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 1de00bbb474..5abf8bed2e4 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -83,13 +83,11 @@ void cpu_get_memory_mapping(CPUState *cpu, 
MemoryMappingList *list,
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-cc->get_memory_mapping(cpu, list, errp);
-}
+if (cc->get_memory_mapping) {
+cc->get_memory_mapping(cpu, list, errp);
+return;
+}
 
-static void cpu_common_get_memory_mapping(CPUState *cpu,
-  MemoryMappingList *list,
-  Error **errp)
-{
 error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");
 }
 
@@ -419,7 +417,6 @@ static void cpu_class_init(ObjectClass *klass, void *data)
 k->parse_features = cpu_common_parse_features;
 k->get_arch_id = cpu_common_get_arch_id;
 k->has_work = cpu_common_has_work;
-k->get_memory_mapping = cpu_common_get_memory_mapping;
 k->gdb_read_register = cpu_common_gdb_read_register;
 k->gdb_write_register = cpu_common_gdb_write_register;
 set_bit(DEVICE_CATEGORY_CPU, dc->categories);
-- 
2.26.2

[PATCH v2 03/17] cpu: Introduce cpu_virtio_is_big_endian()

Introduce the cpu_virtio_is_big_endian() generic helper to avoid
calling CPUClass internal virtio_is_big_endian() one.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h | 9 +
 hw/core/cpu.c | 8 ++--
 hw/virtio/virtio.c| 4 +---
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 2d43f78819f..b12028c3c03 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -602,6 +602,15 @@ hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
  */
 int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs);
 
+/**
+ * cpu_virtio_is_big_endian:
+ * @cpu: CPU
+
+ * Returns %true if a CPU which supports runtime configurable endianness
+ * is currently big-endian.
+ */
+bool cpu_virtio_is_big_endian(CPUState *cpu);
+
 #endif /* CONFIG_USER_ONLY */
 
 /**
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 4dce35f832f..daaff56a79e 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -218,8 +218,13 @@ static int cpu_common_gdb_write_register(CPUState *cpu, 
uint8_t *buf, int reg)
 return 0;
 }
 
-static bool cpu_common_virtio_is_big_endian(CPUState *cpu)
+bool cpu_virtio_is_big_endian(CPUState *cpu)
 {
+CPUClass *cc = CPU_GET_CLASS(cpu);
+
+if (cc->virtio_is_big_endian) {
+return cc->virtio_is_big_endian(cpu);
+}
 return target_words_bigendian();
 }
 
@@ -438,7 +443,6 @@ static void cpu_class_init(ObjectClass *klass, void *data)
 k->write_elf64_note = cpu_common_write_elf64_note;
 k->gdb_read_register = cpu_common_gdb_read_register;
 k->gdb_write_register = cpu_common_gdb_write_register;
-k->virtio_is_big_endian = cpu_common_virtio_is_big_endian;
 set_bit(DEVICE_CATEGORY_CPU, dc->categories);
 dc->realize = cpu_common_realizefn;
 dc->unrealize = cpu_common_unrealizefn;
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 1fd1917ca0f..fe6a4be99e4 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1973,9 +1973,7 @@ static enum virtio_device_endian 
virtio_default_endian(void)
 
 static enum virtio_device_endian virtio_current_cpu_endian(void)
 {
-CPUClass *cc = CPU_GET_CLASS(current_cpu);
-
-if (cc->virtio_is_big_endian(current_cpu)) {
+if (cpu_virtio_is_big_endian(current_cpu)) {
 return VIRTIO_DEVICE_ENDIAN_BIG;
 } else {
 return VIRTIO_DEVICE_ENDIAN_LITTLE;
-- 
2.26.2

[PATCH v2 16/17] cpu: Restrict cpu_paging_enabled / cpu_get_memory_mapping to sysemu

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 960846d2b64..d99d3c830dc 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -427,6 +427,8 @@ static inline void cpu_tb_jmp_cache_clear(CPUState *cpu)
 extern bool mttcg_enabled;
 #define qemu_tcg_mttcg_enabled() (mttcg_enabled)
 
+#if !defined(CONFIG_USER_ONLY)
+
 /**
  * cpu_paging_enabled:
  * @cpu: The CPU whose state is to be inspected.
@@ -444,8 +446,6 @@ bool cpu_paging_enabled(const CPUState *cpu);
 void cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
 Error **errp);
 
-#if !defined(CONFIG_USER_ONLY)
-
 /**
  * cpu_write_elf64_note:
  * @f: pointer to a function that writes memory to a file
-- 
2.26.2

[PATCH v2 04/17] cpu: Directly use cpu_write_elf*() fallback handlers in place

No code directly accesses CPUClass::write_elf*() handlers out
of hw/core/cpu.c (the rest are assignation in target/ code):

  $ git grep -F -- '->write_elf'
  hw/core/cpu.c:157:return (*cc->write_elf32_qemunote)(f, cpu, opaque);
  hw/core/cpu.c:171:return (*cc->write_elf32_note)(f, cpu, cpuid, opaque);
  hw/core/cpu.c:186:return (*cc->write_elf64_qemunote)(f, cpu, opaque);
  hw/core/cpu.c:200:return (*cc->write_elf64_note)(f, cpu, cpuid, opaque);
  hw/core/cpu.c:440:k->write_elf32_qemunote = 
cpu_common_write_elf32_qemunote;
  hw/core/cpu.c:441:k->write_elf32_note = cpu_common_write_elf32_note;
  hw/core/cpu.c:442:k->write_elf64_qemunote = 
cpu_common_write_elf64_qemunote;
  hw/core/cpu.c:443:k->write_elf64_note = cpu_common_write_elf64_note;
  target/arm/cpu.c:2304:cc->write_elf64_note = arm_cpu_write_elf64_note;
  target/arm/cpu.c:2305:cc->write_elf32_note = arm_cpu_write_elf32_note;
  target/i386/cpu.c:7425:cc->write_elf64_note = x86_cpu_write_elf64_note;
  target/i386/cpu.c:7426:cc->write_elf64_qemunote = 
x86_cpu_write_elf64_qemunote;
  target/i386/cpu.c:7427:cc->write_elf32_note = x86_cpu_write_elf32_note;
  target/i386/cpu.c:7428:cc->write_elf32_qemunote = 
x86_cpu_write_elf32_qemunote;
  target/ppc/translate_init.c.inc:10891:cc->write_elf64_note = 
ppc64_cpu_write_elf64_note;
  target/ppc/translate_init.c.inc:10892:cc->write_elf32_note = 
ppc32_cpu_write_elf32_note;
  target/s390x/cpu.c:522:cc->write_elf64_note = s390_cpu_write_elf64_note;

Check the handler presence in place and remove the common fallback code.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/core/cpu.c | 43 ---
 1 file changed, 12 insertions(+), 31 deletions(-)

diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index daaff56a79e..a9ee2c74ec5 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -154,60 +154,45 @@ int cpu_write_elf32_qemunote(WriteCoreDumpFunction f, 
CPUState *cpu,
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
+if (!cc->write_elf32_qemunote) {
+return 0;
+}
 return (*cc->write_elf32_qemunote)(f, cpu, opaque);
 }
 
-static int cpu_common_write_elf32_qemunote(WriteCoreDumpFunction f,
-   CPUState *cpu, void *opaque)
-{
-return 0;
-}
-
 int cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cpu,
  int cpuid, void *opaque)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
+if (!cc->write_elf32_note) {
+return -1;
+}
 return (*cc->write_elf32_note)(f, cpu, cpuid, opaque);
 }
 
-static int cpu_common_write_elf32_note(WriteCoreDumpFunction f,
-   CPUState *cpu, int cpuid,
-   void *opaque)
-{
-return -1;
-}
-
 int cpu_write_elf64_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
  void *opaque)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
+if (!cc->write_elf64_qemunote) {
+return 0;
+}
 return (*cc->write_elf64_qemunote)(f, cpu, opaque);
 }
 
-static int cpu_common_write_elf64_qemunote(WriteCoreDumpFunction f,
-   CPUState *cpu, void *opaque)
-{
-return 0;
-}
-
 int cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cpu,
  int cpuid, void *opaque)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
+if (!cc->write_elf64_note) {
+return -1;
+}
 return (*cc->write_elf64_note)(f, cpu, cpuid, opaque);
 }
 
-static int cpu_common_write_elf64_note(WriteCoreDumpFunction f,
-   CPUState *cpu, int cpuid,
-   void *opaque)
-{
-return -1;
-}
-
-
 static int cpu_common_gdb_read_register(CPUState *cpu, GByteArray *buf, int 
reg)
 {
 return 0;
@@ -437,10 +422,6 @@ static void cpu_class_init(ObjectClass *klass, void *data)
 k->has_work = cpu_common_has_work;
 k->get_paging_enabled = cpu_common_get_paging_enabled;
 k->get_memory_mapping = cpu_common_get_memory_mapping;
-k->write_elf32_qemunote = cpu_common_write_elf32_qemunote;
-k->write_elf32_note = cpu_common_write_elf32_note;
-k->write_elf64_qemunote = cpu_common_write_elf64_qemunote;
-k->write_elf64_note = cpu_common_write_elf64_note;
 k->gdb_read_register = cpu_common_gdb_read_register;
 k->gdb_write_register = cpu_common_gdb_write_register;
 set_bit(DEVICE_CATEGORY_CPU, dc->categories);
-- 
2.26.2

[PATCH v2 02/17] cpu: Un-inline cpu_get_phys_page_debug and cpu_asidx_from_attrs

To be able to later extract the cpu_get_phys_page_debug() and
cpu_asidx_from_attrs() handlers from CPUClass, un-inline them
from "hw/core/cpu.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h | 33 -
 hw/core/cpu.c | 32 
 2 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index c005d3dc2d8..2d43f78819f 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -578,18 +578,8 @@ void cpu_dump_statistics(CPUState *cpu, int flags);
  *
  * Returns: Corresponding physical page address or -1 if no page found.
  */
-static inline hwaddr cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
-   MemTxAttrs *attrs)
-{
-CPUClass *cc = CPU_GET_CLASS(cpu);
-
-if (cc->get_phys_page_attrs_debug) {
-return cc->get_phys_page_attrs_debug(cpu, addr, attrs);
-}
-/* Fallback for CPUs which don't implement the _attrs_ hook */
-*attrs = MEMTXATTRS_UNSPECIFIED;
-return cc->get_phys_page_debug(cpu, addr);
-}
+hwaddr cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
+ MemTxAttrs *attrs);
 
 /**
  * cpu_get_phys_page_debug:
@@ -601,12 +591,7 @@ static inline hwaddr 
cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
  *
  * Returns: Corresponding physical page address or -1 if no page found.
  */
-static inline hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr)
-{
-MemTxAttrs attrs = {};
-
-return cpu_get_phys_page_attrs_debug(cpu, addr, );
-}
+hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 
 /** cpu_asidx_from_attrs:
  * @cpu: CPU
@@ -615,17 +600,7 @@ static inline hwaddr cpu_get_phys_page_debug(CPUState 
*cpu, vaddr addr)
  * Returns the address space index specifying the CPU AddressSpace
  * to use for a memory access with the given transaction attributes.
  */
-static inline int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs)
-{
-CPUClass *cc = CPU_GET_CLASS(cpu);
-int ret = 0;
-
-if (cc->asidx_from_attrs) {
-ret = cc->asidx_from_attrs(cpu, attrs);
-assert(ret < cpu->num_ases && ret >= 0);
-}
-return ret;
-}
+int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs);
 
 #endif /* CONFIG_USER_ONLY */
 
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 00330ba07de..4dce35f832f 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -94,6 +94,38 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,
 error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");
 }
 
+hwaddr cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
+   MemTxAttrs *attrs)
+{
+CPUClass *cc = CPU_GET_CLASS(cpu);
+
+if (cc->get_phys_page_attrs_debug) {
+return cc->get_phys_page_attrs_debug(cpu, addr, attrs);
+}
+/* Fallback for CPUs which don't implement the _attrs_ hook */
+*attrs = MEMTXATTRS_UNSPECIFIED;
+return cc->get_phys_page_debug(cpu, addr);
+}
+
+hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr)
+{
+MemTxAttrs attrs = {};
+
+return cpu_get_phys_page_attrs_debug(cpu, addr, );
+}
+
+int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs)
+{
+CPUClass *cc = CPU_GET_CLASS(cpu);
+int ret = 0;
+
+if (cc->asidx_from_attrs) {
+ret = cc->asidx_from_attrs(cpu, attrs);
+assert(ret < cpu->num_ases && ret >= 0);
+}
+return ret;
+}
+
 /* Resetting the IRQ comes from across the code base so we take the
  * BQL here if we need to.  cpu_interrupt assumes it is held.*/
 void cpu_reset_interrupt(CPUState *cpu, int mask)
-- 
2.26.2

[PATCH v2 00/17] cpu: Introduce SysemuCPUOps structure