Re: [PATCH 4/4] tests/qtest: Replace magic value by NANOSECONDS_PER_SECOND definition

[Thomas Huth]

On 11/10/2020 21.49, Philippe Mathieu-Daudé wrote:
> Use self-explicit NANOSECONDS_PER_SECOND definition instead
> of a magic value.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/qtest/rtc-test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/rtc-test.c b/tests/qtest/rtc-test.c
> index c7af34f6b1b..402ce2c6090 100644
> --- a/tests/qtest/rtc-test.c
> +++ b/tests/qtest/rtc-test.c
> @@ -292,7 +292,7 @@ static void alarm_time(void)
>  break;
>  }
>  
> -clock_step(10);
> +clock_step(NANOSECONDS_PER_SECOND);
>  }
>  
>  g_assert(get_irq(RTC_ISA_IRQ));
> 

Reviewed-by: Thomas Huth 

Re: [PATCH v3 05/16] fuzz: Declare DMA Read callback function

[Paolo Bonzini]

On 11/10/20 17:45, Alexander Bulekov wrote:
> On 201008 0939, Paolo Bonzini wrote:
>> On 21/09/20 04:24, Alexander Bulekov wrote:
>>> This patch declares the fuzz_dma_read_cb function and uses the
>>> preprocessor and linker(weak symbols) to handle these cases:
>>>
>>> When we build softmmu/all with --enable-fuzzing, there should be no
>>> strong symbol defined for fuzz_dma_read_cb, and we link against a weak
>>> stub function.
>>>
>>> When we build softmmu/fuzz with --enable-fuzzing, we link against the
>>> strong symbol in general_fuzz.c
>>>
>>> When we build softmmu/all without --enable-fuzzing, fuzz_dma_read_cb is
>>> an empty, inlined function. As long as we don't call any other functions
>>> when building the arguments, there should be no overhead.
>>
>> Can you move the weak function somewhere in tests/qtest/fuzz instead?
>> Then you don't need an #ifdef because you can add it to specific_fuzz_ss.
> 
> If I understand correctly, specific_fuzz_ss is only used to build
> qemu-fuzz targets. The goal here was to support building qemu-system
> with --enable-fuzzing (ie CONFIG_FUZZ=y), where specific_fuzz isn't
> used. If its too ugly, we could make a stub file under tests/qtest/fuzz
> and add it to specific_ss when: 'CONFIG_FUZZ'.

You're right.

Paolo

Re: [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer

[Paolo Bonzini]

On 11/10/20 17:35, Alexander Bulekov wrote:
>> Instead of always looking for a separator, can you:
>>
>> 1) skip over it if you find it naturally at the end of a command (that
>> is, "FUZZ" is like a comment command)
>>
>> 2) actively search for it only if you stumble upon an unrecognized command?
>>
> What is the end goal? Is it to be able to use the "FUZZ" bytes to fuzz
> devices?

Yes, possibly, and perhaps also using a shorter separator that is easier
to learn.  But if the dictionary is enough to work around the learning
time and it's unlikely that crossover produces inputs like that, I guess
it's okay to have the separator.

Paolo

[PATCH 2/4] hw/pci-host/prep: Remove legacy PReP machine temporary workaround

[Philippe Mathieu-Daudé]

The legacy PReP machine has been removed in commit b2ce76a0730
("hw/ppc/prep: Remove the deprecated "prep" machine and the
OpenHackware BIOS"). This temporary workaround is no more
required, remove it.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/pci-host/prep.c | 32 +++-
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 80dfb67da43..064593d1e52 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -75,7 +75,6 @@ struct PRePPCIState {
 RavenPCIState pci_dev;
 
 int contiguous_map;
-bool is_legacy_prep;
 };
 
 #define BIOS_SIZE (1 * MiB)
@@ -229,24 +228,18 @@ static void raven_pcihost_realizefn(DeviceState *d, Error 
**errp)
 MemoryRegion *address_space_mem = get_system_memory();
 int i;
 
-if (s->is_legacy_prep) {
-for (i = 0; i < PCI_NUM_PINS; i++) {
-sysbus_init_irq(dev, &s->pci_irqs[i]);
-}
-} else {
-/*
- * According to PReP specification section 6.1.6 "System Interrupt
- * Assignments", all PCI interrupts are routed via IRQ 15
- */
-s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
-object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
-&error_fatal);
-qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
-sysbus_init_irq(dev, &s->or_irq->out_irq);
+/*
+ * According to PReP specification section 6.1.6 "System Interrupt
+ * Assignments", all PCI interrupts are routed via IRQ 15.
+ */
+s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
+object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
+&error_fatal);
+qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
+sysbus_init_irq(dev, &s->or_irq->out_irq);
 
-for (i = 0; i < PCI_NUM_PINS; i++) {
-s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
-}
+for (i = 0; i < PCI_NUM_PINS; i++) {
+s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
 }
 
 qdev_init_gpio_in(d, raven_change_gpio, 1);
@@ -403,9 +396,6 @@ static Property raven_pcihost_properties[] = {
 DEFINE_PROP_UINT32("elf-machine", PREPPCIState, pci_dev.elf_machine,
EM_NONE),
 DEFINE_PROP_STRING("bios-name", PREPPCIState, pci_dev.bios_name),
-/* Temporary workaround until legacy prep machine is removed */
-DEFINE_PROP_BOOL("is-legacy-prep", PREPPCIState, is_legacy_prep,
- false),
 DEFINE_PROP_END_OF_LIST()
 };
 
-- 
2.26.2

[PATCH 3/4] hw/pci-host/prep: Fix PCI swizzling in map_irq()

[Philippe Mathieu-Daudé]

In commit a01d8cadadf we changed the number of IRQs to 4 but
forgot to update the map_irq() function. Do it now.

Fixes: a01d8cadadf ("Fix memory corruption ... in PreP emulation")
Signed-off-by: Philippe Mathieu-Daudé 
---
Cc: Jocelyn Mayer 
Cc: Julian Seward 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/pci-host/prep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 064593d1e52..2224135fedb 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -195,7 +195,7 @@ static const MemoryRegionOps raven_io_ops = {
 
 static int raven_map_irq(PCIDevice *pci_dev, int irq_num)
 {
-return (irq_num + (pci_dev->devfn >> 3)) & 1;
+return (irq_num + (pci_dev->devfn >> 3)) & 3;
 }
 
 static void raven_set_irq(void *opaque, int irq_num, int level)
-- 
2.26.2

[PATCH 0/4] hw/pci-host/prep: Fix PCI swizzling in map_irq()

[Philippe Mathieu-Daudé]

Fix a bug in the Raven PCI host, plus few cleanups while here.

Philippe Mathieu-Daudé (4):
  hw/pci-host/prep: Update coding style to make checkpatch.pl happy
  hw/pci-host/prep: Remove legacy PReP machine temporary workaround
  hw/pci-host/prep: Fix PCI swizzling in map_irq()
  docs/system/target-ppc.rst: Update PReP historical information

 docs/system/target-ppc.rst |  8 
 hw/pci-host/prep.c | 32 
 2 files changed, 16 insertions(+), 24 deletions(-)

-- 
2.26.2

[PATCH 4/4] docs/system/target-ppc.rst: Update PReP historical information

[Philippe Mathieu-Daudé]

Link Jocelyn Mayer's web page from the Wayback Machine,
and correct the PReP acronym.

Signed-off-by: Philippe Mathieu-Daudé 
---
Cc: Jocelyn Mayer 
---
 docs/system/target-ppc.rst | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/system/target-ppc.rst b/docs/system/target-ppc.rst
index a2f04c533c2..1f57966771d 100644
--- a/docs/system/target-ppc.rst
+++ b/docs/system/target-ppc.rst
@@ -3,7 +3,7 @@
 PowerPC System emulator
 ---
 
-Use the executable ``qemu-system-ppc`` to simulate a complete 40P (PREP)
+Use the executable ``qemu-system-ppc`` to simulate a complete 40P (PReP)
 or PowerMac PowerPC system.
 
 QEMU emulates the following PowerMac peripherals:
@@ -20,7 +20,7 @@ QEMU emulates the following PowerMac peripherals:
 
 -  VIA-CUDA with ADB keyboard and mouse.
 
-QEMU emulates the following 40P (PREP) peripherals:
+QEMU emulates the following 40P (PReP) peripherals:
 
 -  PCI Bridge
 
@@ -43,5 +43,5 @@ the g3beige and mac99 PowerMac and the 40p machines. OpenBIOS 
is a free
 (GPL v2) portable firmware implementation. The goal is to implement a
 100% IEEE 1275-1994 (referred to as Open Firmware) compliant firmware.
 
-More information is available at
-http://perso.magic.fr/l_indien/qemu-ppc/.
+Archived historical information is available at
+https://web.archive.org/web/20051212135300/http://perso.magic.fr/l_indien/qemu-ppc/.
-- 
2.26.2

[PATCH 1/4] hw/pci-host/prep: Update coding style to make checkpatch.pl happy

[Philippe Mathieu-Daudé]

To make the next commit easier to review, clean this code first.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/pci-host/prep.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index d0323fefb10..80dfb67da43 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -234,8 +234,10 @@ static void raven_pcihost_realizefn(DeviceState *d, Error 
**errp)
 sysbus_init_irq(dev, &s->pci_irqs[i]);
 }
 } else {
-/* According to PReP specification section 6.1.6 "System Interrupt
- * Assignments", all PCI interrupts are routed via IRQ 15 */
+/*
+ * According to PReP specification section 6.1.6 "System Interrupt
+ * Assignments", all PCI interrupts are routed via IRQ 15
+ */
 s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
 object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
 &error_fatal);
-- 
2.26.2

Re: Which qemu change corresponds to RedHat bug 1655408

[Max Reitz]

On 10.10.20 00:54, Jakob Bohm wrote:

[...]

> Theoretically, locking on a raw file needs to be protocol-compatible
> with loop-mounting the same raw file, so if the loop driver doesn't
> probe those magic byte offsets to prevent out-of-order block writes,
> then there is little point for the qemu raw driver to do so.
> 
> This applies to both the loop driver in the host kernel and the loop
> driver on any other machine with file share access to the sane image
> file.

The file access locks aren’t meant to prevent arbitrary other programs
from accessing those specific few bytes, but to prevent concurrently
running qemu processes from accessing the image.  That’s why qemu
doesn’t lock the whole image file, but only a small and very specific
set of bytes.

The problem we originally faced was that some people would run qemu-img
to modify qcow2 images while a VM was running, leading to metadata
corruption and thus data loss.  This is the main reason the locks were
introduced.  However, the problem isn’t limited to qcow2 images.  Even
for raw images, we generally have to prevent e.g. concurrent writes to
the image (from other VMs that might want to use that image) because the
guest won’t be able to deal with that.  Hence why we take locks even on
non-qcow2 images.

> As for upgrading, I will try newer kernels packaged for the Debian
> version used, once the current large batch job has completed, but I
> doubt it will make much difference given the principles I just stated.

I’m not sure whether I understood your paragraphs above wrong, but I
don’t see how they explain why the bug would appear (i.e., why qemu
would fail taking the file lock).  It only seems to explain why it seems
superfluous for qemu to take locks when the loop driver will be the only
concurrent user of the image (presumably because the loop driver just
doesn’t take any locks; which means that qemu should be able to).

Max

Re: [PATCH 2/4] hw/pci-host/prep: Remove legacy PReP machine temporary workaround

[Thomas Huth]

On 12/10/2020 09.19, Philippe Mathieu-Daudé wrote:
> The legacy PReP machine has been removed in commit b2ce76a0730
> ("hw/ppc/prep: Remove the deprecated "prep" machine and the
> OpenHackware BIOS"). This temporary workaround is no more
> required, remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/pci-host/prep.c | 32 +++-
>  1 file changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 80dfb67da43..064593d1e52 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -75,7 +75,6 @@ struct PRePPCIState {
>  RavenPCIState pci_dev;
>  
>  int contiguous_map;
> -bool is_legacy_prep;
>  };
>  
>  #define BIOS_SIZE (1 * MiB)
> @@ -229,24 +228,18 @@ static void raven_pcihost_realizefn(DeviceState *d, 
> Error **errp)
>  MemoryRegion *address_space_mem = get_system_memory();
>  int i;
>  
> -if (s->is_legacy_prep) {
> -for (i = 0; i < PCI_NUM_PINS; i++) {
> -sysbus_init_irq(dev, &s->pci_irqs[i]);
> -}
> -} else {
> -/*
> - * According to PReP specification section 6.1.6 "System Interrupt
> - * Assignments", all PCI interrupts are routed via IRQ 15
> - */
> -s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
> -object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
> -&error_fatal);
> -qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
> -sysbus_init_irq(dev, &s->or_irq->out_irq);
> +/*
> + * According to PReP specification section 6.1.6 "System Interrupt
> + * Assignments", all PCI interrupts are routed via IRQ 15.
> + */

Since you're changing the indentation of these lines anyway, I think you
could also simply squash patch 1 into this one here (just a matter of taste,
though).

> +s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
> +object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
> +&error_fatal);
> +qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
> +sysbus_init_irq(dev, &s->or_irq->out_irq);
>  
> -for (i = 0; i < PCI_NUM_PINS; i++) {
> -s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
> -}
> +for (i = 0; i < PCI_NUM_PINS; i++) {
> +s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
>  }
>  
>  qdev_init_gpio_in(d, raven_change_gpio, 1);
> @@ -403,9 +396,6 @@ static Property raven_pcihost_properties[] = {
>  DEFINE_PROP_UINT32("elf-machine", PREPPCIState, pci_dev.elf_machine,
> EM_NONE),
>  DEFINE_PROP_STRING("bios-name", PREPPCIState, pci_dev.bios_name),
> -/* Temporary workaround until legacy prep machine is removed */
> -DEFINE_PROP_BOOL("is-legacy-prep", PREPPCIState, is_legacy_prep,
> - false),
>  DEFINE_PROP_END_OF_LIST()
>  };
>  
> 

Reviewed-by: Thomas Huth 

Re: [PATCH 4/4] docs/system/target-ppc.rst: Update PReP historical information

[Thomas Huth]

On 12/10/2020 09.19, Philippe Mathieu-Daudé wrote:
> Link Jocelyn Mayer's web page from the Wayback Machine,
> and correct the PReP acronym.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> Cc: Jocelyn Mayer 
> ---
>  docs/system/target-ppc.rst | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/system/target-ppc.rst b/docs/system/target-ppc.rst
> index a2f04c533c2..1f57966771d 100644
> --- a/docs/system/target-ppc.rst
> +++ b/docs/system/target-ppc.rst
> @@ -3,7 +3,7 @@
>  PowerPC System emulator
>  ---
>  
> -Use the executable ``qemu-system-ppc`` to simulate a complete 40P (PREP)
> +Use the executable ``qemu-system-ppc`` to simulate a complete 40P (PReP)
>  or PowerMac PowerPC system.
>  
>  QEMU emulates the following PowerMac peripherals:
> @@ -20,7 +20,7 @@ QEMU emulates the following PowerMac peripherals:
>  
>  -  VIA-CUDA with ADB keyboard and mouse.
>  
> -QEMU emulates the following 40P (PREP) peripherals:
> +QEMU emulates the following 40P (PReP) peripherals:
>  
>  -  PCI Bridge
>  
> @@ -43,5 +43,5 @@ the g3beige and mac99 PowerMac and the 40p machines. 
> OpenBIOS is a free
>  (GPL v2) portable firmware implementation. The goal is to implement a
>  100% IEEE 1275-1994 (referred to as Open Firmware) compliant firmware.
>  
> -More information is available at
> -http://perso.magic.fr/l_indien/qemu-ppc/.
> +Archived historical information is available at
> +https://web.archive.org/web/20051212135300/http://perso.magic.fr/l_indien/qemu-ppc/.

There seems to be a later version available in the archive, too:

https://web.archive.org/web/20080212093125/http://perso.magic.fr/l_indien/qemu-ppc/

... but I think the contents did not change anymore. So:

Reviewed-by: Thomas Huth 

Re: [PATCH v3 05/20] target/mips: Move cpu_mips_get_random() with CP0 helpers

[Luc Michel]

On 22:43 Sat 10 Oct , Philippe Mathieu-Daudé wrote:
> The get_random() helper uses the CP0_Wired register, which is
> unrelated to the CP0_Count register use as timer.
typo: used
> Commit e16fe40c872 ("Move the MIPS CPU timer in a separate file")
> incorrectly moved this get_random() helper with timer specific
> code. Move it back to generic CP0 helpers.
> 
> Reviewed-by: Aleksandar Markovic 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Luc Michel 

> ---
>  target/mips/internal.h   |  2 +-
>  target/mips/cp0_helper.c | 25 +
>  target/mips/cp0_timer.c  | 25 -
>  3 files changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/target/mips/internal.h b/target/mips/internal.h
> index 7f159a9230c..087cabaa6d4 100644
> --- a/target/mips/internal.h
> +++ b/target/mips/internal.h
> @@ -144,6 +144,7 @@ void r4k_helper_tlbr(CPUMIPSState *env);
>  void r4k_helper_tlbinv(CPUMIPSState *env);
>  void r4k_helper_tlbinvf(CPUMIPSState *env);
>  void r4k_invalidate_tlb(CPUMIPSState *env, int idx, int use_extra);
> +uint32_t cpu_mips_get_random(CPUMIPSState *env);
>  
>  void mips_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
>  vaddr addr, unsigned size,
> @@ -209,7 +210,6 @@ void cpu_state_reset(CPUMIPSState *s);
>  void cpu_mips_realize_env(CPUMIPSState *env);
>  
>  /* cp0_timer.c */
> -uint32_t cpu_mips_get_random(CPUMIPSState *env);
>  uint32_t cpu_mips_get_count(CPUMIPSState *env);
>  void cpu_mips_store_count(CPUMIPSState *env, uint32_t value);
>  void cpu_mips_store_compare(CPUMIPSState *env, uint32_t value);
> diff --git a/target/mips/cp0_helper.c b/target/mips/cp0_helper.c
> index de64add038b..12143ac55b9 100644
> --- a/target/mips/cp0_helper.c
> +++ b/target/mips/cp0_helper.c
> @@ -203,6 +203,31 @@ static void sync_c0_entryhi(CPUMIPSState *cpu, int tc)
>  *tcst |= asid;
>  }
>  
> +/* XXX: do not use a global */
> +uint32_t cpu_mips_get_random(CPUMIPSState *env)
> +{
> +static uint32_t seed = 1;
> +static uint32_t prev_idx;
> +uint32_t idx;
> +uint32_t nb_rand_tlb = env->tlb->nb_tlb - env->CP0_Wired;
> +
> +if (nb_rand_tlb == 1) {
> +return env->tlb->nb_tlb - 1;
> +}
> +
> +/* Don't return same value twice, so get another value */
> +do {
> +/*
> + * Use a simple algorithm of Linear Congruential Generator
> + * from ISO/IEC 9899 standard.
> + */
> +seed = 1103515245 * seed + 12345;
> +idx = (seed >> 16) % nb_rand_tlb + env->CP0_Wired;
> +} while (idx == prev_idx);
> +prev_idx = idx;
> +return idx;
> +}
> +
>  /* CP0 helpers */
>  target_ulong helper_mfc0_mvpcontrol(CPUMIPSState *env)
>  {
> diff --git a/target/mips/cp0_timer.c b/target/mips/cp0_timer.c
> index bd7efb152dd..9c38e9da1c8 100644
> --- a/target/mips/cp0_timer.c
> +++ b/target/mips/cp0_timer.c
> @@ -29,31 +29,6 @@
>  
>  #define TIMER_PERIOD 10 /* 10 ns period for 100 Mhz frequency */
>  
> -/* XXX: do not use a global */
> -uint32_t cpu_mips_get_random(CPUMIPSState *env)
> -{
> -static uint32_t seed = 1;
> -static uint32_t prev_idx = 0;
> -uint32_t idx;
> -uint32_t nb_rand_tlb = env->tlb->nb_tlb - env->CP0_Wired;
> -
> -if (nb_rand_tlb == 1) {
> -return env->tlb->nb_tlb - 1;
> -}
> -
> -/* Don't return same value twice, so get another value */
> -do {
> -/*
> - * Use a simple algorithm of Linear Congruential Generator
> - * from ISO/IEC 9899 standard.
> - */
> -seed = 1103515245 * seed + 12345;
> -idx = (seed >> 16) % nb_rand_tlb + env->CP0_Wired;
> -} while (idx == prev_idx);
> -prev_idx = idx;
> -return idx;
> -}
> -
>  /* MIPS R4K timer */
>  static void cpu_mips_timer_update(CPUMIPSState *env)
>  {
> -- 
> 2.26.2
> 

-- 

Re: [PATCH v4 3/4] hw/timer/bcm2835: Support the timer COMPARE registers

[Luc Michel]

On 22:37 Sat 10 Oct , Philippe Mathieu-Daudé wrote:
> This peripheral has 1 free-running timer and 4 compare registers.
> 
> Only the free-running timer is implemented. Add support the
> COMPARE registers (each register is wired to an IRQ).
> 
> Reference: "BCM2835 ARM Peripherals" datasheet [*]
> chapter 12 "System Timer":
> 
>   The System Timer peripheral provides four 32-bit timer channels
>   and a single 64-bit free running counter. Each channel has an
>   output compare register, which is compared against the 32 least
>   significant bits of the free running counter values. When the
>   two values match, the system timer peripheral generates a signal
>   to indicate a match for the appropriate channel. The match signal
>   is then fed into the interrupt controller.
> 
> This peripheral is used since Linux 3.7, commit ee4af5696720
> ("ARM: bcm2835: add system timer").
> 
> [*] 
> https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Luc Michel 

> ---
> v4:
> - Fix arithmetic to correctly get value in future (Richard)
> - Use 32-bit argument value (Richard)
> v3:
> - Only compare 32 least significant bits of the free running
>   counter values (Luc)
> ---
>  include/hw/timer/bcm2835_systmr.h | 11 +--
>  hw/timer/bcm2835_systmr.c | 48 ---
>  hw/timer/trace-events |  6 ++--
>  3 files changed, 44 insertions(+), 21 deletions(-)
> 
> diff --git a/include/hw/timer/bcm2835_systmr.h 
> b/include/hw/timer/bcm2835_systmr.h
> index f15788a78d8..bd3097d746b 100644
> --- a/include/hw/timer/bcm2835_systmr.h
> +++ b/include/hw/timer/bcm2835_systmr.h
> @@ -11,6 +11,7 @@
>  
>  #include "hw/sysbus.h"
>  #include "hw/irq.h"
> +#include "qemu/timer.h"
>  #include "qom/object.h"
>  
>  #define TYPE_BCM2835_SYSTIMER "bcm2835-sys-timer"
> @@ -18,18 +19,24 @@ OBJECT_DECLARE_SIMPLE_TYPE(BCM2835SystemTimerState, 
> BCM2835_SYSTIMER)
>  
>  #define BCM2835_SYSTIMER_COUNT 4
>  
> +typedef struct {
> +unsigned id;
> +QEMUTimer timer;
> +qemu_irq irq;
> +BCM2835SystemTimerState *state;
> +} BCM2835SystemTimerCompare;
> +
>  struct BCM2835SystemTimerState {
>  /*< private >*/
>  SysBusDevice parent_obj;
>  
>  /*< public >*/
>  MemoryRegion iomem;
> -qemu_irq irq;
> -
>  struct {
>  uint32_t ctrl_status;
>  uint32_t compare[BCM2835_SYSTIMER_COUNT];
>  } reg;
> +BCM2835SystemTimerCompare tmr[BCM2835_SYSTIMER_COUNT];
>  };
>  
>  #endif
> diff --git a/hw/timer/bcm2835_systmr.c b/hw/timer/bcm2835_systmr.c
> index b234e83824f..67669a57ff3 100644
> --- a/hw/timer/bcm2835_systmr.c
> +++ b/hw/timer/bcm2835_systmr.c
> @@ -28,20 +28,13 @@ REG32(COMPARE1, 0x10)
>  REG32(COMPARE2, 0x14)
>  REG32(COMPARE3, 0x18)
>  
> -static void bcm2835_systmr_update_irq(BCM2835SystemTimerState *s)
> +static void bcm2835_systmr_timer_expire(void *opaque)
>  {
> -bool enable = !!s->reg.ctrl_status;
> +BCM2835SystemTimerCompare *tmr = opaque;
>  
> -trace_bcm2835_systmr_irq(enable);
> -qemu_set_irq(s->irq, enable);
> -}
> -
> -static void bcm2835_systmr_update_compare(BCM2835SystemTimerState *s,
> -  unsigned timer_index)
> -{
> -/* TODO fow now, since neither Linux nor U-boot use these timers. */
> -qemu_log_mask(LOG_UNIMP, "COMPARE register %u not implemented\n",
> -  timer_index);
> +trace_bcm2835_systmr_timer_expired(tmr->id);
> +tmr->state->reg.ctrl_status |= 1 << tmr->id;
> +qemu_set_irq(tmr->irq, 1);
>  }
>  
>  static uint64_t bcm2835_systmr_read(void *opaque, hwaddr offset,
> @@ -75,19 +68,33 @@ static uint64_t bcm2835_systmr_read(void *opaque, hwaddr 
> offset,
>  }
>  
>  static void bcm2835_systmr_write(void *opaque, hwaddr offset,
> - uint64_t value, unsigned size)
> + uint64_t value64, unsigned size)
>  {
>  BCM2835SystemTimerState *s = BCM2835_SYSTIMER(opaque);
> +int index;
> +uint32_t value = value64;
> +uint32_t triggers_delay_us;
> +uint64_t now;
>  
>  trace_bcm2835_systmr_write(offset, value);
>  switch (offset) {
>  case A_CTRL_STATUS:
>  s->reg.ctrl_status &= ~value; /* Ack */
> -bcm2835_systmr_update_irq(s);
> +for (index = 0; index < ARRAY_SIZE(s->tmr); index++) {
> +if (extract32(value, index, 1)) {
> +trace_bcm2835_systmr_irq_ack(index);
> +qemu_set_irq(s->tmr[index].irq, 0);
> +}
> +}
>  break;
>  case A_COMPARE0 ... A_COMPARE3:
> -s->reg.compare[(offset - A_COMPARE0) >> 2] = value;
> -bcm2835_systmr_update_compare(s, (offset - A_COMPARE0) >> 2);
> +index = (offset - A_COMPARE0) >> 2;
> +s->reg.compare[index] = value;
> +now = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL);
> +/* Compare l

Re: [PATCH] hw/pci-host/grackle: Verify PIC link is properly set

[David Gibson]

On Mon, Oct 12, 2020 at 08:21:41AM +0200, Philippe Mathieu-Daudé wrote:
> On 10/12/20 12:34 AM, David Gibson wrote:
> > On Sun, Oct 11, 2020 at 09:03:32PM +0200, Philippe Mathieu-Daudé wrote:
> > > The Grackle PCI host model expects the interrupt controller
> > > being set, but does not verify it is present. Add a check to
> > > help developers using this model.
> > 
> > I don't think thaqt's very likely, but, sure, applied to ppc-for-5.2
> 
> Do you want I correct the description as:
> "The Grackle PCI host model expects the interrupt controller
> being set, but does not verify it is present.
> A developer basing its implementation on the Grackle model
> might hit this problem. Add a check to help future developers
> using this model as reference."?

No, it's fine.  All I was saying is that the chances of anyone using
Grackle in future seem very low to me.


> 
> > 
> > > 
> > > Signed-off-by: Philippe Mathieu-Daudé 
> > > ---
> > >   hw/pci-host/grackle.c | 4 
> > >   1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
> > > index 57c29b20afb..20361d215ca 100644
> > > --- a/hw/pci-host/grackle.c
> > > +++ b/hw/pci-host/grackle.c
> > > @@ -76,6 +76,10 @@ static void grackle_realize(DeviceState *dev, Error 
> > > **errp)
> > >   GrackleState *s = GRACKLE_PCI_HOST_BRIDGE(dev);
> > >   PCIHostState *phb = PCI_HOST_BRIDGE(dev);
> > > +if (!s->pic) {
> > > +error_setg(errp, TYPE_GRACKLE_PCI_HOST_BRIDGE ": 'pic' link not 
> > > set");
> > > +return;
> > > +}
> > >   phb->bus = pci_register_root_bus(dev, NULL,
> > >pci_grackle_set_irq,
> > >pci_grackle_map_irq,
> > 
> 

-- 
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] MAINTAINERS: Downgrade status of MIPS Boston to "Odd Fixes"

[Thomas Huth]

On 09/10/2020 18.52, Philippe Mathieu-Daudé wrote:
> Paul's Wavecomp email has been bouncing for months. He told us
> he "no longer has access to modern MIPS CPUs or Boston hardware,
> and wouldn't currently have time to spend on them if he did." [1]
> but "perhaps that might change in the future." [2].
> Be fair and downgrade the status of the Boston board to "Odd Fixes"
> (has a maintainer but they don't have time to do much other).

When I read this patch description ("email bouncing"), I wondered why you
did not also update Paul's email address here. Then I saw that you're doing
this in the next patch. So I'd recommend to either scratch the first
sentence of your patch description here, or to merge the two patches.

 Thomas

> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg718739.html
> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg728605.html
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  MAINTAINERS | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2de5943e388..782743b7ef0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1163,7 +1163,7 @@ F: hw/intc/loongson_liointc.c
>  Boston
>  M: Paul Burton 
>  R: Aleksandar Rikalo 
> -S: Maintained
> +S: Odd Fixes
>  F: hw/core/loader-fit.c
>  F: hw/mips/boston.c
>  F: hw/pci-host/xilinx-pcie.c
> 

Re: Re%3A [PATCH v4 1%2F2] hw%2Fwatchdog%3A Implement SBSA watchdog device&In-Reply-To=

[Maxim Uvarov]

> +static uint64_t sbsa_gwdt_rread(void *opaque, hwaddr addr, unsigned int size)
> +{
> +uint32_t ret;
> +
> +if (addr == SBSA_GWDT_WRR) {
> +/* watch refresh read has no effect and returns 0 */
> +ret = 0;
> +} else {
> +qemu_log_mask(LOG_GUEST_ERROR, "bad address in refresh frame read :"
> +

Then it has to be  uint32_t ret = 0;

On Fri, 9 Oct 2020 at 20:51, Peter Maydell  wrote:
>
> On Fri, 9 Oct 2020 at 18:30, Shashi Mallela  wrote:
> >
> > The value being returned here is 0 (initialized to 0 at the beginning of 
> > read function).
> > I have seen similar practices being followed in other qemu implementations 
> > like for example bcm2835_dma_read() in 
> > qemu/hw/dma/bcm2835_dma.c,a9_scu_read() in qemu/hw/misc/a9scu.c.
> >
> > Please confirm if you would still like to add specific value like 
> > 0xdeadbeef for bad read offset.
>
> Judging by the subject line you've mangled the headers on this
> email so I'm not sure exactly what this was a reply to,
> but in general QEMU doesn't do that kind of "return
> specific marker values for bad register offsets". We
> typically log it with a qemu_log_mask(LOG_GUEST_ERROR) and
> return 0 (unless the h/w does something else).
>
> thanks
> -- PMM

Re: [PULL 00/10] migration queue

[Dr. David Alan Gilbert]

* Peter Maydell (peter.mayd...@linaro.org) wrote:
> On Thu, 8 Oct 2020 at 20:13, Dr. David Alan Gilbert (git)
>  wrote:
> >
> > From: "Dr. David Alan Gilbert" 
> >
> > The following changes since commit e64cf4d569f6461d6b9072e00d6e78d0ab8bd4a7:
> >
> >   Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20201008' into 
> > staging (2020-10-08 17:18:46 +0100)
> >
> > are available in the Git repository at:
> >
> >   git://github.com/dagrh/qemu.git tags/pull-migration-20201008a
> >
> > for you to fetch changes up to ee02b58c82749adb486ef2ae7efdc8e05b093cfd:
> >
> >   migration/dirtyrate: present dirty rate only when querying the rate has 
> > completed (2020-10-08 19:57:00 +0100)
> >
> > 
> > Migration and virtiofs pull 2020-10-08
> >
> > v2 (from yesterdays)
> >   Updated types in comparison to fix mingw build
> >   rebased
> >
> > -
> > Migration:
> >   Dirtyrate measurement API cleanup
> >   Postcopy recovery fixes
> >
> > Virtiofsd:
> >   Missing qemu_init_exec_dir call
> >   Support for setting the group on socket creation
> >   Stop a gcc warning
> >   Avoid tempdir in sandboxing
> 
> This seems to hang in 'make check' trying to run
> tests/qtest/migration-test on s390x and ppc, ie
> the big-endian hosts.

OK, I'll give it a try.

Dave

> thanks
> -- PMM
> 
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH V13 8/9] hw/mips: Add Loongson-3 machine support

[Philippe Mathieu-Daudé]

On 10/11/20 4:53 AM, Huacai Chen wrote:

Hi, Philippe,

On Sat, Oct 10, 2020 at 5:09 PM Philippe Mathieu-Daudé  wrote:


Hi Huacai,

On 10/7/20 10:39 AM, Huacai Chen wrote:

Add Loongson-3 based machine support, it use liointc as the interrupt
controler and use GPEX as the pci controller. Currently it can work with
both TCG and KVM.

As the machine model is not based on any exiting physical hardware, the
name of the machine is "loongson3-virt". It may be superseded in future
by a real machine model. If this happens, then a regular deprecation
procedure shall occur for "loongson3-virt" machine.

We now already have a full functional Linux kernel (based on Linux-5.4.x
LTS, the kvm host side and guest side have both been upstream for Linux-
5.9, but Linux-5.9 has not been released yet) here:

https://github.com/chenhuacai/linux

Of course the upstream kernel is also usable (though it is "unstable"
now):

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

How to use QEMU/Loongson-3?
1, Download kernel source from the above URL;
2, Build a kernel with arch/mips/configs/loongson3_defconfig;
3, Boot a Loongson-3A4000 host with this kernel (for KVM mode);
4, Build QEMU-master with this patchset;
5, modprobe kvm (only necessary for KVM mode);
6, Use QEMU with TCG:
 qemu-system-mips64el -M loongson3-virt,accel=tcg -cpu Loongson-3A1000 
-kernel  -append ...
 Use QEMU with KVM:
 qemu-system-mips64el -M loongson3-virt,accel=kvm -cpu Loongson-3A4000 
-kernel  -append ...

 The "-cpu" parameter is optional here and QEMU will use the correct type 
for TCG/KVM automatically.

Signed-off-by: Huacai Chen 
Co-developed-by: Jiaxun Yang 
---
   default-configs/devices/mips64el-softmmu.mak |   1 +
   hw/mips/Kconfig  |  12 +
   hw/mips/loongson3_bootp.c| 162 +++
   hw/mips/loongson3_bootp.h| 225 ++


In a previous version I asked if you could split the bootp* files
(firmware related) in a previous separate patch to ease the review
of the machine part.

>

In previous version you told me to split the fw_cfg to a separate
patch, but split the efi-related helpers to seprate files (not to a
patch). But anyway, I will split them to a patch in the next version.


Sorry if I wasn't clear enough. It is very hard to review a patch
of more than 1000 lines of C code without loosing focus or missing
problems. It explains why big patches might stay unreviewed on the
list. If you can split in smaller logical units, it will be very
helpful.

Peter Maydell rule of thumb is to keep patch below 200 lines.
This is a good limit.






   hw/mips/loongson3_virt.c | 615 
+++
   hw/mips/meson.build  |   1 +
   6 files changed, 1016 insertions(+)

[...]


diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
new file mode 100644
index 000..4e573d6
--- /dev/null
+++ b/hw/mips/loongson3_virt.c
@@ -0,0 +1,615 @@
+/*
+ * Generic Loongson-3 Platform support
+ *
+ * Copyright (c) 2017-2020 Huacai Chen (che...@lemote.com)
+ * Copyright (c) 2017-2020 Jiaxun Yang 
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see .
+ */
+
+/*
+ * Generic virtualized PC Platform based on Loongson-3 CPU (MIPS64R2 with
+ * extensions, 800~2000MHz)
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qemu/units.h"
+#include "qemu/cutils.h"
+#include "qapi/error.h"
+#include "cpu.h"
+#include "elf.h"
+#include "kvm_mips.h"
+#include "hw/boards.h"
+#include "hw/char/serial.h"
+#include "hw/mips/mips.h"
+#include "hw/mips/cpudevs.h"
+#include "hw/mips/fw_cfg.h"
+#include "hw/mips/loongson3_bootp.h"
+#include "hw/misc/unimp.h"
+#include "hw/intc/i8259.h"
+#include "hw/loader.h"
+#include "hw/isa/superio.h"
+#include "hw/pci/msi.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_host.h"
+#include "hw/pci-host/gpex.h"
+#include "hw/rtc/mc146818rtc.h"
+#include "hw/usb.h"
+#include "net/net.h"
+#include "exec/address-spaces.h"
+#include "sysemu/kvm.h"
+#include "sysemu/qtest.h"
+#include "sysemu/reset.h"
+#include "sysemu/runstate.h"
+#include "qemu/log.h"
+#include "qemu/error-report.h"
+
+#define PM_CNTL_MODE  0x10
+
+#define LOONGSON_MAX_VCPUS  16
+
+#define LOONGSON3_BIOSNAME "bios_loongson3.bin"


Where is that file?

>

I don't know the policy of QEMU's bina

Re: [PATCH 2/4] hw/pci-host/prep: Remove legacy PReP machine temporary workaround

[Philippe Mathieu-Daudé]

On 10/12/20 9:28 AM, Thomas Huth wrote:

On 12/10/2020 09.19, Philippe Mathieu-Daudé wrote:

The legacy PReP machine has been removed in commit b2ce76a0730
("hw/ppc/prep: Remove the deprecated "prep" machine and the
OpenHackware BIOS"). This temporary workaround is no more
required, remove it.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/pci-host/prep.c | 32 +++-
  1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 80dfb67da43..064593d1e52 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -75,7 +75,6 @@ struct PRePPCIState {
  RavenPCIState pci_dev;
  
  int contiguous_map;

-bool is_legacy_prep;
  };
  
  #define BIOS_SIZE (1 * MiB)

@@ -229,24 +228,18 @@ static void raven_pcihost_realizefn(DeviceState *d, Error 
**errp)
  MemoryRegion *address_space_mem = get_system_memory();
  int i;
  
-if (s->is_legacy_prep) {

-for (i = 0; i < PCI_NUM_PINS; i++) {
-sysbus_init_irq(dev, &s->pci_irqs[i]);
-}
-} else {
-/*
- * According to PReP specification section 6.1.6 "System Interrupt
- * Assignments", all PCI interrupts are routed via IRQ 15
- */
-s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
-object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
-&error_fatal);
-qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
-sysbus_init_irq(dev, &s->or_irq->out_irq);
+/*
+ * According to PReP specification section 6.1.6 "System Interrupt
+ * Assignments", all PCI interrupts are routed via IRQ 15.
+ */


Since you're changing the indentation of these lines anyway, I think you
could also simply squash patch 1 into this one here (just a matter of taste,
though).


Then the diff is not trivial to review :/




+s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
+object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
+&error_fatal);
+qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
+sysbus_init_irq(dev, &s->or_irq->out_irq);
  
-for (i = 0; i < PCI_NUM_PINS; i++) {

-s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
-}
+for (i = 0; i < PCI_NUM_PINS; i++) {
+s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
  }
  
  qdev_init_gpio_in(d, raven_change_gpio, 1);

@@ -403,9 +396,6 @@ static Property raven_pcihost_properties[] = {
  DEFINE_PROP_UINT32("elf-machine", PREPPCIState, pci_dev.elf_machine,
 EM_NONE),
  DEFINE_PROP_STRING("bios-name", PREPPCIState, pci_dev.bios_name),
-/* Temporary workaround until legacy prep machine is removed */
-DEFINE_PROP_BOOL("is-legacy-prep", PREPPCIState, is_legacy_prep,
- false),
  DEFINE_PROP_END_OF_LIST()
  };
  



Reviewed-by: Thomas Huth 


Thanks!

Re: [PATCH v3 12/20] hw/mips/r4k: Explicit CPU frequency is 200 MHz

[Philippe Mathieu-Daudé]

Hi Huacai,

On 10/11/20 5:52 AM, chen huacai wrote:

Hi, Philippe,

On Sun, Oct 11, 2020 at 4:43 AM Philippe Mathieu-Daudé  wrote:


Since its introduction in commit 6af0bf9c7c3,
the 'r4k' machine runs at 200 MHz.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/mips/r4k.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/mips/r4k.c b/hw/mips/r4k.c
index 3487013a4a1..e64687b505a 100644
--- a/hw/mips/r4k.c
+++ b/hw/mips/r4k.c
@@ -37,6 +37,7 @@
  #include "sysemu/reset.h"
  #include "sysemu/runstate.h"
  #include "qemu/error-report.h"
+#include "hw/qdev-clock.h"

  #define MAX_IDE_BUS 2

@@ -184,6 +185,7 @@ void mips_r4k_init(MachineState *machine)
  int bios_size;
  MIPSCPU *cpu;
  CPUMIPSState *env;
+Clock *cpuclk;
  ResetData *reset_info;
  int i;
  qemu_irq *i8259;
@@ -193,7 +195,11 @@ void mips_r4k_init(MachineState *machine)
  int be;

  /* init CPUs */
-cpu = MIPS_CPU(cpu_create(machine->cpu_type));
+cpu = MIPS_CPU(object_new(machine->cpu_type));
+cpuclk = clock_new(OBJECT(machine), "cpu-refclk");
+clock_set_hz(cpuclk, 2); /* 200 MHz */
+qdev_connect_clock_in(DEVICE(cpu), "clk-in", cpuclk);
+qdev_realize(DEVICE(cpu), NULL, &error_abort);

>

Can we add a new parameter to cpu_create() and set the freq in the core code?


Adding a new parameter seems a good idea.

Both maintainers of the core code are reluctant to add
a CPU clock to the core code, see:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg747589.html
https://www.mail-archive.com/qemu-devel@nongnu.org/msg747612.html
Which is why I restricted that to the MIPS CPUs.

On ARM, Damien started to use clocks on the Zynq SoC (merged):
https://www.mail-archive.com/qemu-devel@nongnu.org/msg694604.html
Luc is working on adding a clock manager to the Broadcom SoC:
https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg02840.html

I also started converting one UART devices:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg727972.html

So IMO the core code will soon require this. Maybe we will add it
during the next development window.

Meanwhile I can add a mips_cpu_create_with_clock() in preparation
of cpu_create_with_clock().



Huacai

  env = &cpu->env;

  reset_info = g_malloc0(sizeof(ResetData));
--
2.26.2

[PATCH v4 2/3] qga: add implementation of guest-get-disks for Linux

[Tomáš Golembiovský]

The command lists all disks (real and virtual) as well as disk
partitions. For each disk the list of dependent disks is also listed and
/dev path is used as a handle so it can be matched with "name" field of
other returned disk entries. For disk partitions the "dependents" list
is populated with the the parent device for easier tracking of
hierarchy.

Example output:
{
  "return": [
...
{
  "name": "/dev/dm-0",
  "partition": false,
  "dependents": [
"/dev/sda2"
  ],
  "alias": "luks-7062202e-5b9b-433e-81e8-6628c40da9f7"
},
{
  "name": "/dev/sda2",
  "partition": true,
  "dependents": [
"/dev/sda"
  ]
},
{
  "name": "/dev/sda",
  "partition": false,
  "address": {
"serial": "SAMSUNG_MZ7LN512HCHP-000L1_S1ZKNXAG822493",
"bus-type": "sata",
...
"dev": "/dev/sda",
"target": 0
  },
  "dependents": []
},
...
  ]
}

Signed-off-by: Tomáš Golembiovský 
---
 qga/commands-posix.c | 296 +--
 1 file changed, 285 insertions(+), 11 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 422144bcff..14683dfbd5 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1150,13 +1150,27 @@ static void build_guest_fsinfo_for_virtual_device(char 
const *syspath,
 closedir(dir);
 }
 
+static bool is_disk_virtual(const char *devpath, Error **errp)
+{
+g_autofree char *syspath = realpath(devpath, NULL);
+
+if (!syspath) {
+error_setg_errno(errp, errno, "realpath(\"%s\")", devpath);
+return false;
+}
+return strstr(syspath, "/devices/virtual/block/") != NULL;
+}
+
 /* Dispatch to functions for virtual/real device */
 static void build_guest_fsinfo_for_device(char const *devpath,
   GuestFilesystemInfo *fs,
   Error **errp)
 {
-char *syspath = realpath(devpath, NULL);
+ERRP_GUARD();
+g_autofree char *syspath = NULL;
+bool is_virtual = false;
 
+syspath = realpath(devpath, NULL);
 if (!syspath) {
 error_setg_errno(errp, errno, "realpath(\"%s\")", devpath);
 return;
@@ -1167,16 +1181,281 @@ static void build_guest_fsinfo_for_device(char const 
*devpath,
 }
 
 g_debug("  parse sysfs path '%s'", syspath);
-
-if (strstr(syspath, "/devices/virtual/block/")) {
+is_virtual = is_disk_virtual(syspath, errp);
+if (*errp != NULL) {
+return;
+}
+if (is_virtual) {
 build_guest_fsinfo_for_virtual_device(syspath, fs, errp);
 } else {
 build_guest_fsinfo_for_real_device(syspath, fs, errp);
 }
+}
+
+#ifdef CONFIG_LIBUDEV
+
+/*
+ * Wrapper around build_guest_fsinfo_for_device() for getting just
+ * the disk address.
+ */
+static GuestDiskAddress *get_disk_address(const char *syspath, Error **errp)
+{
+g_autoptr(GuestFilesystemInfo) fs = NULL;
+
+fs = g_new0(GuestFilesystemInfo, 1);
+build_guest_fsinfo_for_device(syspath, fs, errp);
+if (fs->disk != NULL) {
+return g_steal_pointer(&fs->disk->value);
+}
+return NULL;
+}
+
+static char *get_alias_for_syspath(const char *syspath)
+{
+struct udev *udev = NULL;
+struct udev_device *udevice = NULL;
+char *ret = NULL;
+
+udev = udev_new();
+if (udev == NULL) {
+g_debug("failed to query udev");
+goto out;
+}
+udevice = udev_device_new_from_syspath(udev, syspath);
+if (udevice == NULL) {
+g_debug("failed to query udev for path: %s", syspath);
+goto out;
+} else {
+const char *alias = udev_device_get_property_value(
+udevice, "DM_NAME");
+/*
+ * NULL means there was an error and empty string means there is no
+ * alias. In case of no alias we return NULL instead of empty string.
+ */
+if (alias == NULL) {
+g_debug("failed to query udev for device alias for: %s",
+syspath);
+} else if (*alias != 0) {
+ret = g_strdup(alias);
+}
+}
+
+out:
+udev_unref(udev);
+udev_device_unref(udevice);
+return ret;
+}
+
+static char *get_device_for_syspath(const char *syspath)
+{
+struct udev *udev = NULL;
+struct udev_device *udevice = NULL;
+char *ret = NULL;
+
+udev = udev_new();
+if (udev == NULL) {
+g_debug("failed to query udev");
+goto out;
+}
+udevice = udev_device_new_from_syspath(udev, syspath);
+if (udevice == NULL) {
+g_debug("failed to query udev for path: %s", syspath);
+goto out;
+} else {
+ret = g_strdup(udev_device_get_devnode(udevice));
+}
+
+out:
+udev_unref(udev);
+udev_device_unref(udevice);
+return ret;
+}
+
+static void get_disk_deps(const char *disk_dir, GuestDiskInfo *disk)
+{
+g_autofree char *deps_dir = NULL;
+const gchar *dep;
+GDir *dp_deps = NULL;
+
+

[PATCH v4 3/3] qga: add implementation of guest-get-disks for Windows

[Tomáš Golembiovský]

The command lists all the physical disk drives. Unlike for Linux
partitions and virtual volumes are not listed.

Example output:

{
  "return": [
{
  "name": ".\\PhysicalDrive0",
  "partition": false,
  "address": {
"serial": "QM1",
"bus-type": "sata",
...
  },
  "dependents": []
}
  ]
}

Signed-off-by: Tomáš Golembiovský 
---
 qga/commands-win32.c | 107 ---
 1 file changed, 101 insertions(+), 6 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 0dd16315d7..b3e3682005 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -979,6 +979,101 @@ out:
 return list;
 }
 
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+ERRP_GUARD();
+GuestDiskInfoList *new = NULL, *ret = NULL;
+HDEVINFO dev_info;
+SP_DEVICE_INTERFACE_DATA dev_iface_data;
+int i;
+
+dev_info = SetupDiGetClassDevs(&GUID_DEVINTERFACE_DISK, 0, 0,
+DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
+if (dev_info == INVALID_HANDLE_VALUE) {
+error_setg_win32(errp, GetLastError(), "failed to get device tree");
+return NULL;
+}
+
+g_debug("enumerating devices");
+dev_iface_data.cbSize = sizeof(SP_DEVICE_INTERFACE_DATA);
+for (i = 0;
+SetupDiEnumDeviceInterfaces(dev_info, NULL, &GUID_DEVINTERFACE_DISK,
+i, &dev_iface_data);
+i++) {
+GuestDiskAddress *address = NULL;
+GuestDiskInfo *disk = NULL;
+Error *local_err = NULL;
+g_autofree PSP_DEVICE_INTERFACE_DETAIL_DATA
+pdev_iface_detail_data = NULL;
+STORAGE_DEVICE_NUMBER sdn;
+HANDLE dev_file;
+DWORD size = 0;
+BOOL result;
+int attempt;
+
+g_debug("  getting device path");
+for (attempt = 0, result = FALSE; attempt < 2 && !result; attempt++) {
+result = SetupDiGetDeviceInterfaceDetail(dev_info,
+&dev_iface_data, pdev_iface_detail_data, size, &size, NULL);
+if (result) {
+break;
+}
+if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
+pdev_iface_detail_data = g_realloc(pdev_iface_detail_data,
+size);
+pdev_iface_detail_data->cbSize =
+sizeof(*pdev_iface_detail_data);
+} else {
+g_debug("failed to get device interface details");
+break;
+}
+}
+if (!result) {
+g_debug("skipping device");
+continue;
+}
+
+g_debug("  device: %s", pdev_iface_detail_data->DevicePath);
+dev_file = CreateFile(pdev_iface_detail_data->DevicePath, 0,
+FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, NULL);
+if (!DeviceIoControl(dev_file, IOCTL_STORAGE_GET_DEVICE_NUMBER,
+NULL, 0, &sdn, sizeof(sdn), &size, NULL)) {
+CloseHandle(dev_file);
+debug_error("failed to get storage device number");
+continue;
+}
+CloseHandle(dev_file);
+
+disk = g_new0(GuestDiskInfo, 1);
+disk->name = g_strdup_printf(".\\PhysicalDrive%lu",
+sdn.DeviceNumber);
+
+g_debug("  number: %lu", sdn.DeviceNumber);
+address = g_malloc0(sizeof(GuestDiskAddress));
+address->has_dev = true;
+address->dev = g_strdup(disk->name);
+get_single_disk_info(sdn.DeviceNumber, address, &local_err);
+if (local_err) {
+g_debug("failed to get disk info: %s",
+error_get_pretty(local_err));
+error_free(local_err);
+qapi_free_GuestDiskAddress(address);
+address = NULL;
+} else {
+disk->address = address;
+disk->has_address = true;
+}
+
+new = g_malloc0(sizeof(GuestDiskInfoList));
+new->value = disk;
+new->next = ret;
+ret = new;
+}
+
+SetupDiDestroyDeviceInfoList(dev_info);
+return ret;
+}
+
 #else
 
 static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
@@ -986,6 +1081,12 @@ static GuestDiskAddressList *build_guest_disk_info(char 
*guid, Error **errp)
 return NULL;
 }
 
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+error_setg(errp, QERR_UNSUPPORTED);
+return NULL;
+}
+
 #endif /* CONFIG_QGA_NTDDSCSI */
 
 static GuestFilesystemInfo *build_guest_fsinfo(char *guid, Error **errp)
@@ -2457,9 +2558,3 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
 }
 return head;
 }
-
-GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
-{
-error_setg(errp, QERR_UNSUPPORTED);
-return NULL;
-}
-- 
2.28.0

[PATCH v4 0/3] qga: add command guest-get-disks

[Tomáš Golembiovský]

Adds command to list disks of the VM.

The patch does compile on master but to work properly it requires changes to
qemu-ga by Thomas Huth in series: Allow guest-get-fsinfo also for non-PCI
devices.

v4:
- 2/3: don't leak syspath in build_guest_fsinfo_for_device()
- 3/3:
  - added ERRP_GUARD
  - don't loop forever when qga fails to get device information details

v3:
- renamed "slaves" field to "dependents"
- comments from Marc and Daniel
- 2/3: factored out pieces of code into separate functions

v2:
- added new struct in API to handle the information
- split changes into several patches
- for Linux list also slaves, partitions and virtual disks so that the disk
  hierarchy is preserved
- fixed issues pointed out by Michael

Tomáš Golembiovský (3):
  qga: add command guest-get-disks
  qga: add implementation of guest-get-disks for Linux
  qga: add implementation of guest-get-disks for Windows

 qga/commands-posix.c | 290 ++-
 qga/commands-win32.c | 101 +++
 qga/qapi-schema.json |  31 +
 3 files changed, 417 insertions(+), 5 deletions(-)

-- 
2.28.0

[PATCH v4 1/3] qga: add command guest-get-disks

[Tomáš Golembiovský]

Add API and stubs for new guest-get-disks command.

The command guest-get-fsinfo can be used to list information about disks
and partitions but it is limited only to mounted disks with filesystem.
This new command should allow listing information about disks of the VM
regardles whether they are mounted or not. This can be usefull for
management applications for mapping virtualized devices or pass-through
devices to device names in the guest OS.

Signed-off-by: Tomáš Golembiovský 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Marc-André Lureau 
---
 qga/commands-posix.c |  6 ++
 qga/commands-win32.c |  6 ++
 qga/qapi-schema.json | 31 +++
 3 files changed, 43 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 3bffee99d4..422144bcff 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -3051,3 +3051,9 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
 
 return NULL;
 }
+
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+error_setg(errp, QERR_UNSUPPORTED);
+return NULL;
+}
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 0c3c05484f..0dd16315d7 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2457,3 +2457,9 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
 }
 return head;
 }
+
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+error_setg(errp, QERR_UNSUPPORTED);
+return NULL;
+}
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index cec98c7e06..1ba8f19efc 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -865,6 +865,37 @@
'bus': 'int', 'target': 'int', 'unit': 'int',
'*serial': 'str', '*dev': 'str'} }
 
+##
+# @GuestDiskInfo:
+#
+# @name: device node (Linux) or device UNC (Windows)
+# @partition: whether this is a partition or disk
+# @dependents: list of dependent devices; e.g. for LVs of the LVM this will
+#  hold the list of PVs, for LUKS encrypted volume this will
+#  contain the disk where the volume is placed. (Linux)
+# @address: disk address information (only for non-virtual devices)
+# @alias: optional alias assigned to the disk, on Linux this is a name assigned
+# by device mapper
+#
+# Since 5.2
+##
+{ 'struct': 'GuestDiskInfo',
+  'data': {'name': 'str', 'partition': 'bool', 'dependents': ['str'],
+   '*address': 'GuestDiskAddress', '*alias': 'str'} }
+
+##
+# @guest-get-disks:
+#
+# Returns: The list of disks in the guest. For Windows these are only the
+#  physical disks. On Linux these are all root block devices of
+#  non-zero size including e.g. removable devices, loop devices,
+#  NBD, etc.
+#
+# Since: 5.2
+##
+{ 'command': 'guest-get-disks',
+  'returns': ['GuestDiskInfo'] }
+
 ##
 # @GuestFilesystemInfo:
 #
-- 
2.28.0

[Bug 1637511] Re: Armitage crashes KVM guest with Kali2016.2 for QXL video

[Thomas Huth]

Ok, thanks for your answer, so I'm closing this ticket now.

** Changed in: qemu
   Status: Incomplete => Fix Released

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

Title:
  Armitage crashes KVM guest with Kali2016.2 for QXL video

Status in QEMU:
  Fix Released

Bug description:
  I recently got a strange bug which seems to be related to qemu-kvm and
  QXL. I came here via the hints of the KVM web-site for KVM/qemu bug
  tracking. But, I am not sure whether this is the right bug-tracker at
  all. Please advise me if I placed the report wrongly.

  I installed Kali2016.2 as a KVM guest on a Opensuse Leap 42.1 host
  (fully updated). The KVM guest machine was configured to use a spice
  display and QXL video. Everything OK with the installation with the
  exception of one major application with a Java interface - Armitage.

  Armitage is correctly configured and starts (with some minor Java
  errors) and opens its interface (msf console, target window  etc.)
  Trying to open the 2 specific menu points "Hosts" or "Attack" in the
  menu bar leads to something very strange: The screen flickers, then
  the whole login session is stopped and a standard login window opens.
  This happens independently of the setting for the type of Armitage
  target window (graphical or table like)

  Why do I report this bug here? 
  Because it happens with the QXL graphical video interface ONLY - not with 
video=vga or vmvga ! Neither does the bug occur when Armitage is started in a 
ssh (-X) session from the host. 

  So, it is closely related to qemu-kvm AND QXL and the Java interaction
  with both.

  I really wonder what in the world can make 2 specific menu points of a
  Java application crash a KVM guest and restart a login shell in Kali
  only when QXL is used?

  qemu-kvm version : 2.3.1
  Kernel version of OS LEAP 42.1: Linux 4.1.31-30-default   

  I have described the bug also to the Kali people - see
  https://bugs.kali.org/view.php?id=3698

  Please inform me what further data are required - if this is relevant
  in this bug-tracker at all.

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

Re: [PATCH] hw/pci-host/grackle: Verify PIC link is properly set

[Mark Cave-Ayland]

On 11/10/2020 20:03, Philippe Mathieu-Daudé wrote:

> The Grackle PCI host model expects the interrupt controller
> being set, but does not verify it is present. Add a check to
> help developers using this model.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/pci-host/grackle.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
> index 57c29b20afb..20361d215ca 100644
> --- a/hw/pci-host/grackle.c
> +++ b/hw/pci-host/grackle.c
> @@ -76,6 +76,10 @@ static void grackle_realize(DeviceState *dev, Error **errp)
>  GrackleState *s = GRACKLE_PCI_HOST_BRIDGE(dev);
>  PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>  
> +if (!s->pic) {
> +error_setg(errp, TYPE_GRACKLE_PCI_HOST_BRIDGE ": 'pic' link not 
> set");
> +return;
> +}
>  phb->bus = pci_register_root_bus(dev, NULL,
>   pci_grackle_set_irq,
>   pci_grackle_map_irq,

Reviewing this now, my feeling is that both grackle and uninorth are doing the 
wrong
thing here by passing in a link to the PIC - certainly if I had written this 
more
recently than I originally did, I would simply use qdev_init_gpio_out() for the 
IRQ
lines and do the wiring during machine init.

I've got a few other things to look at first, but I'll post a patchset later 
which I
think will improve this for both Mac machines.


ATB,

Mark.

Re: [PATCH] target/sparc/int32_helper: Remove duplicated 'Tag Overflow' entry

[Mark Cave-Ayland]

On 11/10/2020 21:01, Philippe Mathieu-Daudé wrote:

> Commit 0b09be2b2f ("Nicer debug output for exceptions") added
> twice the same "Tag Overflow" entry, remove the extra one.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  target/sparc/int32_helper.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/target/sparc/int32_helper.c b/target/sparc/int32_helper.c
> index 9a71e1abd87..ba63c739c1e 100644
> --- a/target/sparc/int32_helper.c
> +++ b/target/sparc/int32_helper.c
> @@ -50,7 +50,6 @@ static const char * const excp_names[0x80] = {
>  [TT_EXTINT | 0xd] = "External Interrupt 13",
>  [TT_EXTINT | 0xe] = "External Interrupt 14",
>  [TT_EXTINT | 0xf] = "External Interrupt 15",
> -[TT_TOVF] = "Tag Overflow",
>  [TT_CODE_ACCESS] = "Instruction Access Error",
>  [TT_DATA_ACCESS] = "Data Access Error",
>  [TT_DIV_ZERO] = "Division By Zero",

Reviewed-by: Mark Cave-Ayland 


ATB,

Mark.

Re: [PATCH 2/4] hw/pci-host/prep: Remove legacy PReP machine temporary workaround

[Mark Cave-Ayland]

On 12/10/2020 08:19, Philippe Mathieu-Daudé wrote:

> The legacy PReP machine has been removed in commit b2ce76a0730
> ("hw/ppc/prep: Remove the deprecated "prep" machine and the
> OpenHackware BIOS"). This temporary workaround is no more
> required, remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/pci-host/prep.c | 32 +++-
>  1 file changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 80dfb67da43..064593d1e52 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -75,7 +75,6 @@ struct PRePPCIState {
>  RavenPCIState pci_dev;
>  
>  int contiguous_map;
> -bool is_legacy_prep;
>  };
>  
>  #define BIOS_SIZE (1 * MiB)
> @@ -229,24 +228,18 @@ static void raven_pcihost_realizefn(DeviceState *d, 
> Error **errp)
>  MemoryRegion *address_space_mem = get_system_memory();
>  int i;
>  
> -if (s->is_legacy_prep) {
> -for (i = 0; i < PCI_NUM_PINS; i++) {
> -sysbus_init_irq(dev, &s->pci_irqs[i]);
> -}
> -} else {
> -/*
> - * According to PReP specification section 6.1.6 "System Interrupt
> - * Assignments", all PCI interrupts are routed via IRQ 15
> - */
> -s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
> -object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
> -&error_fatal);
> -qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
> -sysbus_init_irq(dev, &s->or_irq->out_irq);
> +/*
> + * According to PReP specification section 6.1.6 "System Interrupt
> + * Assignments", all PCI interrupts are routed via IRQ 15.
> + */
> +s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
> +object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
> +&error_fatal);
> +qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
> +sysbus_init_irq(dev, &s->or_irq->out_irq);
>  
> -for (i = 0; i < PCI_NUM_PINS; i++) {
> -s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
> -}
> +for (i = 0; i < PCI_NUM_PINS; i++) {
> +s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
>  }
>  
>  qdev_init_gpio_in(d, raven_change_gpio, 1);
> @@ -403,9 +396,6 @@ static Property raven_pcihost_properties[] = {
>  DEFINE_PROP_UINT32("elf-machine", PREPPCIState, pci_dev.elf_machine,
> EM_NONE),
>  DEFINE_PROP_STRING("bios-name", PREPPCIState, pci_dev.bios_name),
> -/* Temporary workaround until legacy prep machine is removed */
> -DEFINE_PROP_BOOL("is-legacy-prep", PREPPCIState, is_legacy_prep,
> - false),
>  DEFINE_PROP_END_OF_LIST()
>  };

Yes indeed, this workaround is certainly no longer needed.

Reviewed-by: Mark Cave-Ayland 


ATB,

Mark.

Re: [PATCH 3/4] hw/pci-host/prep: Fix PCI swizzling in map_irq()

[Mark Cave-Ayland]

On 12/10/2020 08:19, Philippe Mathieu-Daudé wrote:

> In commit a01d8cadadf we changed the number of IRQs to 4 but
> forgot to update the map_irq() function. Do it now.
> 
> Fixes: a01d8cadadf ("Fix memory corruption ... in PreP emulation")
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> Cc: Jocelyn Mayer 
> Cc: Julian Seward 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/pci-host/prep.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 064593d1e52..2224135fedb 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -195,7 +195,7 @@ static const MemoryRegionOps raven_io_ops = {
>  
>  static int raven_map_irq(PCIDevice *pci_dev, int irq_num)
>  {
> -return (irq_num + (pci_dev->devfn >> 3)) & 1;
> +return (irq_num + (pci_dev->devfn >> 3)) & 3;
>  }
>  
>  static void raven_set_irq(void *opaque, int irq_num, int level)

It feels like this should also have a corresponding change in OpenBIOS for
consistency, even though technically because of the OR on IRQ 15 it doesn't 
really
matter. The relevant part in OpenBIOS can be found here:
https://git.qemu.org/?p=openbios.git;a=blob;f=drivers/pci.c;h=34ae69a907b6312a3a7ab218afe8ba9efded1df7;hb=7f28286f5cb1ca682e3ba0a8706d8884f12bc49e#l2001
and in particular this section:

/* Use the same "physical" routing as QEMU's raven_map_irq() although
   ultimately all 4 PCI interrupts are ORd to IRQ 15 as indicated
   by the PReP specification */
props[(*ncells)++] = arch->irqs[((intno - 1) + (addr >> 11)) & 1];


ATB,

Mark.

Re: [PATCH 1/4] hw/pci-host/prep: Update coding style to make checkpatch.pl happy

[Mark Cave-Ayland]

On 12/10/2020 08:19, Philippe Mathieu-Daudé wrote:

> To make the next commit easier to review, clean this code first.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/pci-host/prep.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index d0323fefb10..80dfb67da43 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -234,8 +234,10 @@ static void raven_pcihost_realizefn(DeviceState *d, 
> Error **errp)
>  sysbus_init_irq(dev, &s->pci_irqs[i]);
>  }
>  } else {
> -/* According to PReP specification section 6.1.6 "System Interrupt
> - * Assignments", all PCI interrupts are routed via IRQ 15 */
> +/*
> + * According to PReP specification section 6.1.6 "System Interrupt
> + * Assignments", all PCI interrupts are routed via IRQ 15
> + */
>  s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
>  object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
>  &error_fatal);

Reviewed-by: Mark Cave-Ayland 


ATB,

Mark.

Re: [PATCH v2 2/4] Makefile: Require GNU make 3.82+

[Thomas Huth]

On 25/08/2020 22.27, Roman Bolshakov wrote:
> QEMU build fails with cryptic messages if make is too old:
> 
>   Makefile.ninja:2655: *** multiple target patterns.  Stop.
> 
> To avoid the confusion it's worth to fail the build right away and print
> a friendly error message.
> 
> Reviewed-by: Daniel P. Berrangé 
> Signed-off-by: Roman Bolshakov 
> ---
>  Makefile | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 81794d5c34..b4ebf3e30f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -4,6 +4,11 @@ ifneq ($(words $(subst :, ,$(CURDIR))), 1)
>$(error main directory cannot contain spaces nor colons)
>  endif
>  
> +ifeq ($(filter undefine,$(value .FEATURES)),)
> +$(error Unsupported Make version: $(MAKE_VERSION). \
> +Please use GNU Make 3.82 or above)
> +endif
> +
>  # Always point to the root of the build tree (needs GNU make).
>  BUILD_DIR=$(CURDIR)

Reviewed-by: Thomas Huth 

Re: [PATCH v2 4/4] configure: Test if $make actually exists

[Thomas Huth]

On 25/08/2020 22.27, Roman Bolshakov wrote:
> configure doesn't detect if $make is installed on the build host.
> This is also helpful for hosts where an alias for make is used, i.e.
> configure would fail if gmake is not present on macOS.
> 
> Reviewed-by: Daniel P. Berrangé 
> Signed-off-by: Roman Bolshakov 
> ---
>  configure | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/configure b/configure
> index 664084992b..9230832da2 100755
> --- a/configure
> +++ b/configure
> @@ -2029,6 +2029,10 @@ if test -z "$python"
>  then
>  error_exit "Python not found. Use --python=/path/to/python"
>  fi
> +if ! has "$make"
> +then
> +error_exit "GNU make ($make) not found"
> +fi
>  
>  # Note that if the Python conditional here evaluates True we will exit
>  # with status 1 which is a shell 'false' value.
> 

Reviewed-by: Thomas Huth 

[Bug 1849644] Re: QEMU VNC websocket proxy requires non-standard 'binary' subprotocol

[Launchpad Bug Tracker]

This bug was fixed in the package qemu - 1:4.2-3ubuntu6.7

---
qemu (1:4.2-3ubuntu6.7) focal; urgency=medium

  * d/p/ubuntu/lp-1882774-*: add newer EPYC processor types (LP: #1887490)
  * d/p/u/lp-1896751-exec-rom_reset-Free-rom-data-during-inmigrate-skip.patch:
fix reboot after migration (LP: #1896751)
  * d/p/u/lp-1849644-io-channel-websock-treat-binary-and-no-sub-protocol-.patch:
fix websocket compatibility with newer versions of noVNC (LP: #1849644)

 -- Christian Ehrhardt   Mon, 27 Jul
2020 11:45:26 +0200

** Changed in: qemu (Ubuntu Focal)
   Status: Fix Committed => Fix Released

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

Title:
  QEMU VNC websocket proxy requires non-standard 'binary' subprotocol

Status in QEMU:
  Fix Released
Status in qemu package in Ubuntu:
  Fix Released
Status in qemu source package in Focal:
  Fix Released

Bug description:
  [Impact]

   * The exact details of the protocol/subprotocal was slightly unclear
 between the projects. So qemu ended up insisting on "binary" being
 used but newer noVNC clients no more used it.

   * Qemu got fixed in 5.0 to be more tolerant and accept an empty sub-
 protocol as well. This SRU backports that fix to Focal.

  [Test Case]

   * Without the fix the following will "Failed to connect", but with
  the fix it will work.

  $ sudo apt install qemu-system-x86
  # will only boot into a failing bootloader, but that is enough
  $ qemu-system-x86_64 -vnc :0,websocket
  # We need version 1.2 or later, so use the snap
  $ snap install novnc
  $ novnc --vnc localhost:5700
  Connect browser to http://:6080/vnc.html
  Click "Connect"

   * Cross check with an older noVNC (e.g. the one in Focal) if the 
 connectivity still works on those as well

 - Reminders when switching between the noVNC implementations
   - always refresh the browser with all clear ctrl+alt+f5
   - start/stop the snapped one via snap.novnc.novncsvc.service

  [Regression Potential]

   * This is exclusive to the functionality of noVNC, so regressions would 
 have to be expected in there. The tests try to exercise the basics, but
 e.g. Openstack would be a major product using 

  [Other Info]
   
   * The noVNC in Focal itself does not yet have the offending change, but
 we want the qemu in focal to be connecteable from ~any type of client


  ---


  
  When running a machine using "-vnc" and the "websocket" option QEMU seems to 
require the subprotocol called 'binary'. This subprotocol does not exist in the 
WebSocket specification. In fact it has never existed in the spec, in one of 
the very early drafts of WebSockets it was briefly mentioned but it never made 
it to a final version.

  When the WebSocket server requires a non-standard subprotocol any
  WebSocket client that works correctly won't be able to connect.

  One example of such a client is noVNC, it tells the server that it
  doesn't want to use any subprotocol. QEMU's WebSocket proxy doesn't
  let noVNC connect. If noVNC is modified to ask for 'binary' it will
  work, this is, however, incorrect behavior.

  Looking at the code in "io/channel-websock.c" it seems it's quite
  hard-coded to binary:

  Look at line 58 and 433 here:
  https://git.qemu.org/?p=qemu.git;a=blob;f=io/channel-websock.c

  This code has to be made more dynamic, and shouldn't require binary.

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

[Bug 1849644] Update Released

[Łukasz Zemczak]

The verification of the Stable Release Update for qemu has completed
successfully and the package is now being released to -updates.
Subsequently, the Ubuntu Stable Release Updates Team is being
unsubscribed and will not receive messages about this bug report.  In
the event that you encounter a regression using the package from
-updates please report a new bug using ubuntu-bug and tag the bug report
regression-update so we can easily find any regressions.

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

Title:
  QEMU VNC websocket proxy requires non-standard 'binary' subprotocol

Status in QEMU:
  Fix Released
Status in qemu package in Ubuntu:
  Fix Released
Status in qemu source package in Focal:
  Fix Released

Bug description:
  [Impact]

   * The exact details of the protocol/subprotocal was slightly unclear
 between the projects. So qemu ended up insisting on "binary" being
 used but newer noVNC clients no more used it.

   * Qemu got fixed in 5.0 to be more tolerant and accept an empty sub-
 protocol as well. This SRU backports that fix to Focal.

  [Test Case]

   * Without the fix the following will "Failed to connect", but with
  the fix it will work.

  $ sudo apt install qemu-system-x86
  # will only boot into a failing bootloader, but that is enough
  $ qemu-system-x86_64 -vnc :0,websocket
  # We need version 1.2 or later, so use the snap
  $ snap install novnc
  $ novnc --vnc localhost:5700
  Connect browser to http://:6080/vnc.html
  Click "Connect"

   * Cross check with an older noVNC (e.g. the one in Focal) if the 
 connectivity still works on those as well

 - Reminders when switching between the noVNC implementations
   - always refresh the browser with all clear ctrl+alt+f5
   - start/stop the snapped one via snap.novnc.novncsvc.service

  [Regression Potential]

   * This is exclusive to the functionality of noVNC, so regressions would 
 have to be expected in there. The tests try to exercise the basics, but
 e.g. Openstack would be a major product using 

  [Other Info]
   
   * The noVNC in Focal itself does not yet have the offending change, but
 we want the qemu in focal to be connecteable from ~any type of client


  ---


  
  When running a machine using "-vnc" and the "websocket" option QEMU seems to 
require the subprotocol called 'binary'. This subprotocol does not exist in the 
WebSocket specification. In fact it has never existed in the spec, in one of 
the very early drafts of WebSockets it was briefly mentioned but it never made 
it to a final version.

  When the WebSocket server requires a non-standard subprotocol any
  WebSocket client that works correctly won't be able to connect.

  One example of such a client is noVNC, it tells the server that it
  doesn't want to use any subprotocol. QEMU's WebSocket proxy doesn't
  let noVNC connect. If noVNC is modified to ask for 'binary' it will
  work, this is, however, incorrect behavior.

  Looking at the code in "io/channel-websock.c" it seems it's quite
  hard-coded to binary:

  Look at line 58 and 433 here:
  https://git.qemu.org/?p=qemu.git;a=blob;f=io/channel-websock.c

  This code has to be made more dynamic, and shouldn't require binary.

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

Re: [PATCH 3/5] MAINTAINERS: Downgrade status of MIPS Boston to "Odd Fixes"

[Philippe Mathieu-Daudé]

On 10/12/20 9:40 AM, Thomas Huth wrote:

On 09/10/2020 18.52, Philippe Mathieu-Daudé wrote:

Paul's Wavecomp email has been bouncing for months. He told us
he "no longer has access to modern MIPS CPUs or Boston hardware,
and wouldn't currently have time to spend on them if he did." [1]
but "perhaps that might change in the future." [2].
Be fair and downgrade the status of the Boston board to "Odd Fixes"
(has a maintainer but they don't have time to do much other).


When I read this patch description ("email bouncing"), I wondered why you
did not also update Paul's email address here. Then I saw that you're doing
this in the next patch. So I'd recommend to either scratch the first
sentence of your patch description here, or to merge the two patches.


I don't want to scratch it here because this justifies the downgrade.
I'll merge with the next, hoping nobody will complain these are
unrelated changes and has to be split =)



  Thomas


[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg718739.html
[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg728605.html

Signed-off-by: Philippe Mathieu-Daudé 
---
  MAINTAINERS | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2de5943e388..782743b7ef0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1163,7 +1163,7 @@ F: hw/intc/loongson_liointc.c
  Boston
  M: Paul Burton 
  R: Aleksandar Rikalo 
-S: Maintained
+S: Odd Fixes
  F: hw/core/loader-fit.c
  F: hw/mips/boston.c
  F: hw/pci-host/xilinx-pcie.c

[PATCH v4 02/21] qdev-monitor: Display frequencies scaled to SI unit

[Philippe Mathieu-Daudé]

Since commit 9f2ff99c7f2 ("qdev-monitor: print the device's clock
with info qtree") we can display the clock frequencies in the
monitor. Use the recently introduced freq_to_str() to display
the frequencies using the closest SI unit (human friendlier).

Before:

  (qemu) info qtree
  [...]
  dev: xilinx,zynq_slcr, id ""
clock-in "ps_clk" freq_hz=3.33e+07
mmio f800/1000

After:

  dev: xilinx,zynq_slcr, id ""
clock-in "ps_clk" freq_hz=33.3 MHz
mmio f800/1000

Reviewed-by: Luc Michel 
Reviewed-by: Damien Hedde 
Reviewed-by: Alistair Francis 
Signed-off-by: Philippe Mathieu-Daudé 
---
 qdev-monitor.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/qdev-monitor.c b/qdev-monitor.c
index e9b7228480d..a0301cfca81 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -747,11 +747,13 @@ static void qdev_print(Monitor *mon, DeviceState *dev, 
int indent)
 }
 }
 QLIST_FOREACH(ncl, &dev->clocks, node) {
-qdev_printf("clock-%s%s \"%s\" freq_hz=%e\n",
+g_autofree char *freq = NULL;
+
+freq = freq_to_str(clock_get_hz(ncl->clock));
+qdev_printf("clock-%s%s \"%s\" freq_hz=%s\n",
 ncl->output ? "out" : "in",
 ncl->alias ? " (alias)" : "",
-ncl->name,
-CLOCK_PERIOD_TO_HZ(1.0 * clock_get(ncl->clock)));
+ncl->name, freq);
 }
 class = object_get_class(OBJECT(dev));
 do {
-- 
2.26.2

[PATCH v4 00/21] hw/mips: Set CPU frequency

[Philippe Mathieu-Daudé]

Since v3:
- Introduced mips_cpu_create_with_clock() helper (Huacai)
- Added R-b tags

Since v2:
- Renamed "clk" -> "clk-in"
- Renamed "cpuclk-out -> "cpu-refclk"

Missing review: patches 7, 10-13, 15-21

~~~

All the MIPS cores emulated by QEMU provides the Coproc#0
'Count' register which can be used as a free running timer.

Since it's introduction in 2005 this timer uses a fixed
frequency of 100 MHz (for a CPU freq of 200 MHz).
While this is not an issue with Linux guests, it makes
some firmwares behave incorrectly.

The Clock API allow propagating clocks. It is particularly
useful when hardware dynamicly changes clock frequencies.

To be able to model such MIPS hardware, we need to refactor
the MIPS hardware code to handle clocks.

This series is organized as follow:

1/ qdev/clock patches already reviewed but not merged

2/ refactor the CP0 timer period to allow dynamic changes

3/ MIPS CPU get an optional input clock

4/ set correct CPU frequencies to all boards

I used a MIPSsim test suggested by Thomas.
The test is available on the list:
https://mid.mail-archive.com/20200928171539.788309-17-f4bug@amsat.org

Possible follow up:
- QOM'ify the GIC
- let the GIC handle dynamic clock changes

Regards,

Phil.

Luc Michel (1):
  hw/core/clock: add the clock_new helper function

Philippe Mathieu-Daudé (20):
  util/cutils: Introduce freq_to_str() to display Hertz units
  qdev-monitor: Display frequencies scaled to SI unit
  hw/qdev-clock: Display error hint when clock is missing from device
  target/mips: Move cpu_mips_get_random() with CP0 helpers
  target/mips/cp0_timer: Explicit unit in variable name
  target/mips/cp0_timer: Document TIMER_PERIOD origin
  target/mips: Move cp0_count_ns to CPUMIPSState
  target/mips/cpu: Calculate the CP0 timer period using the CPU
frequency
  target/mips/cpu: Make cp0_count_rate a property
  target/mips/cpu: Allow the CPU to use dynamic frequencies
  target/mips/cpu: Introduce mips_cpu_create_with_clock() helper
  hw/mips/r4k: Explicit CPU frequency is 200 MHz
  hw/mips/fuloong2e: Set CPU frequency to 533 MHz
  hw/mips/mipssim: Correct CPU frequency
  hw/mips/jazz: Correct CPU frequencies
  hw/mips/cps: Expose input clock and connect it to CPU cores
  hw/mips/boston: Set CPU frequency to 1 GHz
  hw/mips/malta: Set CPU frequency to 320 MHz
  hw/mips/cps: Do not allow use without input clock
  target/mips/cpu: Display warning when CPU is used without input clock

 include/hw/clock.h   | 13 ++
 include/hw/mips/cps.h|  2 ++
 include/qemu/cutils.h| 12 +
 target/mips/cpu.h| 26 +++
 target/mips/internal.h   |  2 +-
 hw/core/clock.c  | 15 +++
 hw/core/qdev-clock.c | 11 
 hw/mips/boston.c | 13 ++
 hw/mips/cps.c|  9 +++
 hw/mips/fuloong2e.c  |  7 +-
 hw/mips/jazz.c   | 15 ++-
 hw/mips/malta.c  | 19 +++---
 hw/mips/mipssim.c| 11 +++-
 hw/mips/r4k.c|  7 +-
 qdev-monitor.c   |  8 +++---
 target/mips/cp0_helper.c | 25 +++
 target/mips/cp0_timer.c  | 51 ++---
 target/mips/cpu.c| 54 +++-
 util/cutils.c| 14 +++
 19 files changed, 264 insertions(+), 50 deletions(-)

-- 
2.26.2

[PATCH v4 03/21] hw/qdev-clock: Display error hint when clock is missing from device

[Philippe Mathieu-Daudé]

Instead of directly aborting, display a hint to help the developer
figure out the problem (likely trying to connect a clock to a device
pre-dating the Clock API, thus not expecting clocks).

Reviewed-by: Luc Michel 
Reviewed-by: Damien Hedde 
Reviewed-by: Edgar E. Iglesias 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/core/qdev-clock.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
index 47ecb5b4fae..6a9a340d0fb 100644
--- a/hw/core/qdev-clock.c
+++ b/hw/core/qdev-clock.c
@@ -12,6 +12,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "hw/qdev-clock.h"
 #include "hw/qdev-core.h"
 #include "qapi/error.h"
@@ -153,6 +154,11 @@ Clock *qdev_get_clock_in(DeviceState *dev, const char 
*name)
 assert(name);
 
 ncl = qdev_get_clocklist(dev, name);
+if (!ncl) {
+error_report("Can not find clock-in '%s' for device type '%s'",
+ name, object_get_typename(OBJECT(dev)));
+abort();
+}
 assert(!ncl->output);
 
 return ncl->clock;
@@ -165,6 +171,11 @@ Clock *qdev_get_clock_out(DeviceState *dev, const char 
*name)
 assert(name);
 
 ncl = qdev_get_clocklist(dev, name);
+if (!ncl) {
+error_report("Can not find clock-out '%s' for device type '%s'",
+ name, object_get_typename(OBJECT(dev)));
+abort();
+}
 assert(ncl->output);
 
 return ncl->clock;
-- 
2.26.2

[PATCH v4 01/21] util/cutils: Introduce freq_to_str() to display Hertz units

[Philippe Mathieu-Daudé]

Introduce freq_to_str() to convert frequency values in human
friendly units using the SI units for Hertz.

Suggested-by: Luc Michel 
Reviewed-by: Alistair Francis 
Reviewed-by: Luc Michel 
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/qemu/cutils.h | 12 
 util/cutils.c | 14 ++
 2 files changed, 26 insertions(+)

diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index 3a86ec0321e..4bbf4834ea5 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -158,6 +158,18 @@ int qemu_strtosz_metric(const char *nptr, const char 
**end, uint64_t *result);
 
 char *size_to_str(uint64_t val);
 
+/**
+ * freq_to_str:
+ * @freq_hz: frequency to stringify
+ *
+ * Return human readable string for frequency @freq_hz.
+ * Use SI units like KHz, MHz, and so forth.
+ *
+ * The caller is responsible for releasing the value returned
+ * with g_free() after use.
+ */
+char *freq_to_str(uint64_t freq_hz);
+
 /* used to print char* safely */
 #define STR_OR_NULL(str) ((str) ? (str) : "null")
 
diff --git a/util/cutils.c b/util/cutils.c
index 8da34e04b0b..be4e43a9eff 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -885,6 +885,20 @@ char *size_to_str(uint64_t val)
 return g_strdup_printf("%0.3g %sB", (double)val / div, suffixes[i]);
 }
 
+char *freq_to_str(uint64_t freq_hz)
+{
+static const char *const suffixes[] = { "", "K", "M", "G", "T", "P", "E" };
+double freq = freq_hz;
+size_t idx = 0;
+
+while (freq >= 1000.0 && idx < ARRAY_SIZE(suffixes)) {
+freq /= 1000.0;
+idx++;
+}
+
+return g_strdup_printf("%0.3g %sHz", freq, suffixes[idx]);
+}
+
 int qemu_pstrcmp0(const char **str1, const char **str2)
 {
 return g_strcmp0(*str1, *str2);
-- 
2.26.2

[PATCH v4 07/21] target/mips/cp0_timer: Document TIMER_PERIOD origin

[Philippe Mathieu-Daudé]

TIMER_PERIOD value of '10 ns' can be explained looking at
commit 6af0bf9c7c3doc, where the CPU frequency is 200 MHz
and CP0 default count rate is half the frequency of the
CPU. Document that.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/mips/cp0_timer.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/target/mips/cp0_timer.c b/target/mips/cp0_timer.c
index 5194c967ae3..6fec5fe0ff7 100644
--- a/target/mips/cp0_timer.c
+++ b/target/mips/cp0_timer.c
@@ -27,7 +27,17 @@
 #include "sysemu/kvm.h"
 #include "internal.h"
 
-#define TIMER_PERIOD 10 /* 10 ns period for 100 Mhz frequency */
+/*
+ * Since commit 6af0bf9c7c3 this model assumes a CPU clocked at 200MHz
+ * and a CP0 timer running at half the clock of the CPU (cp0_count_rate = 2).
+ *
+ * TIMER_FREQ_HZ = CPU_FREQ_HZ / CP0_COUNT_RATE = 200 MHz / 2 = 100 MHz
+ *
+ * TIMER_PERIOD_NS = 1 / TIMER_FREQ_HZ = 10 ns
+ */
+#define CPU_FREQ_HZ_DEFAULT 2
+#define CP0_COUNT_RATE_DEFAULT  2
+#define TIMER_PERIOD10  /* 1 / (CPU_FREQ_HZ / CP0_COUNT_RATE) */
 
 /* MIPS R4K timer */
 static void cpu_mips_timer_update(CPUMIPSState *env)
-- 
2.26.2

[PATCH v4 04/21] hw/core/clock: add the clock_new helper function

[Philippe Mathieu-Daudé]

From: Luc Michel 

This function creates a clock and parents it to another object with a given
name. It calls clock_setup_canonical_path before returning the new
clock.

This function is useful to create clocks in devices when one doesn't
want to expose it at the qdev level (as an input or an output).

Suggested-by: Philippe Mathieu-Daudé 
Signed-off-by: Luc Michel 
Message-Id: <20201010135759.437903-4-...@lmichel.fr>
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/clock.h | 13 +
 hw/core/clock.c| 15 +++
 2 files changed, 28 insertions(+)

diff --git a/include/hw/clock.h b/include/hw/clock.h
index d357594df99..cbc5e6ced1e 100644
--- a/include/hw/clock.h
+++ b/include/hw/clock.h
@@ -90,6 +90,19 @@ extern const VMStateDescription vmstate_clock;
  */
 void clock_setup_canonical_path(Clock *clk);
 
+/**
+ * clock_new:
+ * @parent: the clock parent
+ * @name: the clock object name
+ *
+ * Helper function to create a new clock and parent it to @parent. There is no
+ * need to call clock_setup_canonical_path on the returned clock as it is done
+ * by this function.
+ *
+ * @return the newly created clock
+ */
+Clock *clock_new(Object *parent, const char *name);
+
 /**
  * clock_set_callback:
  * @clk: the clock to register the callback into
diff --git a/hw/core/clock.c b/hw/core/clock.c
index 7066282f7b9..f866717a835 100644
--- a/hw/core/clock.c
+++ b/hw/core/clock.c
@@ -23,6 +23,21 @@ void clock_setup_canonical_path(Clock *clk)
 clk->canonical_path = object_get_canonical_path(OBJECT(clk));
 }
 
+Clock *clock_new(Object *parent, const char *name)
+{
+Object *obj;
+Clock *clk;
+
+obj = object_new(TYPE_CLOCK);
+object_property_add_child(parent, name, obj);
+object_unref(obj);
+
+clk = CLOCK(obj);
+clock_setup_canonical_path(clk);
+
+return clk;
+}
+
 void clock_set_callback(Clock *clk, ClockCallback *cb, void *opaque)
 {
 clk->callback = cb;
-- 
2.26.2

[PATCH v4 05/21] target/mips: Move cpu_mips_get_random() with CP0 helpers

[Philippe Mathieu-Daudé]

The get_random() helper uses the CP0_Wired register, which is
unrelated to the CP0_Count register used as timer.
Commit e16fe40c872 ("Move the MIPS CPU timer in a separate file")
incorrectly moved this get_random() helper with timer specific
code. Move it back to generic CP0 helpers.

Reviewed-by: Aleksandar Markovic 
Reviewed-by: Luc Michel 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/mips/internal.h   |  2 +-
 target/mips/cp0_helper.c | 25 +
 target/mips/cp0_timer.c  | 25 -
 3 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/target/mips/internal.h b/target/mips/internal.h
index 7f159a9230c..087cabaa6d4 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -144,6 +144,7 @@ void r4k_helper_tlbr(CPUMIPSState *env);
 void r4k_helper_tlbinv(CPUMIPSState *env);
 void r4k_helper_tlbinvf(CPUMIPSState *env);
 void r4k_invalidate_tlb(CPUMIPSState *env, int idx, int use_extra);
+uint32_t cpu_mips_get_random(CPUMIPSState *env);
 
 void mips_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
 vaddr addr, unsigned size,
@@ -209,7 +210,6 @@ void cpu_state_reset(CPUMIPSState *s);
 void cpu_mips_realize_env(CPUMIPSState *env);
 
 /* cp0_timer.c */
-uint32_t cpu_mips_get_random(CPUMIPSState *env);
 uint32_t cpu_mips_get_count(CPUMIPSState *env);
 void cpu_mips_store_count(CPUMIPSState *env, uint32_t value);
 void cpu_mips_store_compare(CPUMIPSState *env, uint32_t value);
diff --git a/target/mips/cp0_helper.c b/target/mips/cp0_helper.c
index de64add038b..12143ac55b9 100644
--- a/target/mips/cp0_helper.c
+++ b/target/mips/cp0_helper.c
@@ -203,6 +203,31 @@ static void sync_c0_entryhi(CPUMIPSState *cpu, int tc)
 *tcst |= asid;
 }
 
+/* XXX: do not use a global */
+uint32_t cpu_mips_get_random(CPUMIPSState *env)
+{
+static uint32_t seed = 1;
+static uint32_t prev_idx;
+uint32_t idx;
+uint32_t nb_rand_tlb = env->tlb->nb_tlb - env->CP0_Wired;
+
+if (nb_rand_tlb == 1) {
+return env->tlb->nb_tlb - 1;
+}
+
+/* Don't return same value twice, so get another value */
+do {
+/*
+ * Use a simple algorithm of Linear Congruential Generator
+ * from ISO/IEC 9899 standard.
+ */
+seed = 1103515245 * seed + 12345;
+idx = (seed >> 16) % nb_rand_tlb + env->CP0_Wired;
+} while (idx == prev_idx);
+prev_idx = idx;
+return idx;
+}
+
 /* CP0 helpers */
 target_ulong helper_mfc0_mvpcontrol(CPUMIPSState *env)
 {
diff --git a/target/mips/cp0_timer.c b/target/mips/cp0_timer.c
index bd7efb152dd..9c38e9da1c8 100644
--- a/target/mips/cp0_timer.c
+++ b/target/mips/cp0_timer.c
@@ -29,31 +29,6 @@
 
 #define TIMER_PERIOD 10 /* 10 ns period for 100 Mhz frequency */
 
-/* XXX: do not use a global */
-uint32_t cpu_mips_get_random(CPUMIPSState *env)
-{
-static uint32_t seed = 1;
-static uint32_t prev_idx = 0;
-uint32_t idx;
-uint32_t nb_rand_tlb = env->tlb->nb_tlb - env->CP0_Wired;
-
-if (nb_rand_tlb == 1) {
-return env->tlb->nb_tlb - 1;
-}
-
-/* Don't return same value twice, so get another value */
-do {
-/*
- * Use a simple algorithm of Linear Congruential Generator
- * from ISO/IEC 9899 standard.
- */
-seed = 1103515245 * seed + 12345;
-idx = (seed >> 16) % nb_rand_tlb + env->CP0_Wired;
-} while (idx == prev_idx);
-prev_idx = idx;
-return idx;
-}
-
 /* MIPS R4K timer */
 static void cpu_mips_timer_update(CPUMIPSState *env)
 {
-- 
2.26.2

[PATCH v4 06/21] target/mips/cp0_timer: Explicit unit in variable name

[Philippe Mathieu-Daudé]

Name variables holding nanoseconds with the '_ns' suffix.

Reviewed-by: Aleksandar Markovic 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/mips/cp0_timer.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/target/mips/cp0_timer.c b/target/mips/cp0_timer.c
index 9c38e9da1c8..5194c967ae3 100644
--- a/target/mips/cp0_timer.c
+++ b/target/mips/cp0_timer.c
@@ -32,13 +32,14 @@
 /* MIPS R4K timer */
 static void cpu_mips_timer_update(CPUMIPSState *env)
 {
-uint64_t now, next;
+uint64_t now_ns, next_ns;
 uint32_t wait;
 
-now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-wait = env->CP0_Compare - env->CP0_Count - (uint32_t)(now / TIMER_PERIOD);
-next = now + (uint64_t)wait * TIMER_PERIOD;
-timer_mod(env->timer, next);
+now_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+wait = env->CP0_Compare - env->CP0_Count -
+   (uint32_t)(now_ns / TIMER_PERIOD);
+next_ns = now_ns + (uint64_t)wait * TIMER_PERIOD;
+timer_mod(env->timer, next_ns);
 }
 
 /* Expire the timer.  */
@@ -56,16 +57,16 @@ uint32_t cpu_mips_get_count(CPUMIPSState *env)
 if (env->CP0_Cause & (1 << CP0Ca_DC)) {
 return env->CP0_Count;
 } else {
-uint64_t now;
+uint64_t now_ns;
 
-now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+now_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 if (timer_pending(env->timer)
-&& timer_expired(env->timer, now)) {
+&& timer_expired(env->timer, now_ns)) {
 /* The timer has already expired.  */
 cpu_mips_timer_expire(env);
 }
 
-return env->CP0_Count + (uint32_t)(now / TIMER_PERIOD);
+return env->CP0_Count + (uint32_t)(now_ns / TIMER_PERIOD);
 }
 }
 
-- 
2.26.2

[PATCH v4 09/21] target/mips/cpu: Calculate the CP0 timer period using the CPU frequency

[Philippe Mathieu-Daudé]

The CP0 timer period is a function of the CPU frequency.
Start using the default values, which will be replaced by
properties in the next commits.

Reviewed-by: Jiaxun Yang 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/mips/cpu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 84b727fefa8..46188139b7b 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -144,13 +144,13 @@ static void mips_cpu_disas_set_info(CPUState *s, 
disassemble_info *info)
  */
 #define CPU_FREQ_HZ_DEFAULT 2
 #define CP0_COUNT_RATE_DEFAULT  2
-#define TIMER_PERIOD_DEFAULT10  /* 1 / (CPU_FREQ_HZ / CP0_COUNT_RATE) */
 
 static void mips_cp0_period_set(MIPSCPU *cpu)
 {
 CPUMIPSState *env = &cpu->env;
 
-env->cp0_count_ns = TIMER_PERIOD_DEFAULT;
+env->cp0_count_ns = muldiv64(NANOSECONDS_PER_SECOND, 
CP0_COUNT_RATE_DEFAULT,
+ CPU_FREQ_HZ_DEFAULT);
 }
 
 static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.26.2

[PATCH v4 11/21] target/mips/cpu: Allow the CPU to use dynamic frequencies

[Philippe Mathieu-Daudé]

Use the Clock API and let the CPU object have an input clock.

If no clock is connected, keep using the default frequency of
200 MHz used since the introduction of the 'r4k' machine in
commit 6af0bf9c7c3.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/mips/cpu.h |  4 
 target/mips/cpu.c | 10 --
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index baeceb892ef..062a4ba6225 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -4,6 +4,7 @@
 #include "cpu-qom.h"
 #include "exec/cpu-defs.h"
 #include "fpu/softfloat-types.h"
+#include "hw/clock.h"
 #include "mips-defs.h"
 
 #define TCG_GUEST_DEFAULT_MO (0)
@@ -1151,6 +1152,8 @@ struct CPUMIPSState {
 /**
  * MIPSCPU:
  * @env: #CPUMIPSState
+ * @clock: this CPU input clock (may be connected
+ * to an output clock from another device).
  * @cp0_count_rate: rate at which the coprocessor 0 counter increments
  *
  * A MIPS CPU.
@@ -1160,6 +1163,7 @@ struct MIPSCPU {
 CPUState parent_obj;
 /*< public >*/
 
+Clock *clock;
 CPUNegativeOffsetState neg;
 CPUMIPSState env;
 /*
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 461edfe22b7..3deb0245e7c 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -27,6 +27,7 @@
 #include "sysemu/kvm.h"
 #include "exec/exec-all.h"
 #include "hw/qdev-properties.h"
+#include "hw/qdev-clock.h"
 
 static void mips_cpu_set_pc(CPUState *cs, vaddr value)
 {
@@ -144,8 +145,8 @@ static void mips_cp0_period_set(MIPSCPU *cpu)
 {
 CPUMIPSState *env = &cpu->env;
 
-env->cp0_count_ns = muldiv64(NANOSECONDS_PER_SECOND, cpu->cp0_count_rate,
- CPU_FREQ_HZ_DEFAULT);
+env->cp0_count_ns = cpu->cp0_count_rate
+* clock_get_ns(MIPS_CPU(cpu)->clock);
 }
 
 static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
@@ -155,6 +156,10 @@ static void mips_cpu_realizefn(DeviceState *dev, Error 
**errp)
 MIPSCPUClass *mcc = MIPS_CPU_GET_CLASS(dev);
 Error *local_err = NULL;
 
+if (!clock_get(cpu->clock)) {
+/* Initialize the frequency in case the clock remains unconnected. */
+clock_set_hz(cpu->clock, CPU_FREQ_HZ_DEFAULT);
+}
 mips_cp0_period_set(cpu);
 
 cpu_exec_realizefn(cs, &local_err);
@@ -178,6 +183,7 @@ static void mips_cpu_initfn(Object *obj)
 MIPSCPUClass *mcc = MIPS_CPU_GET_CLASS(obj);
 
 cpu_set_cpustate_pointers(cpu);
+cpu->clock = qdev_init_clock_in(DEVICE(obj), "clk-in", NULL, cpu);
 env->cpu_model = mcc->cpu_def;
 }
 
-- 
2.26.2

[PATCH v4 10/21] target/mips/cpu: Make cp0_count_rate a property

[Philippe Mathieu-Daudé]

Since not all CPU implementations use a cores use a CP0 timer
at half the frequency of the CPU, make this variable a property.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/mips/cpu.h |  9 +
 target/mips/cpu.c | 19 +++
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 085a88e9550..baeceb892ef 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -1151,6 +1151,7 @@ struct CPUMIPSState {
 /**
  * MIPSCPU:
  * @env: #CPUMIPSState
+ * @cp0_count_rate: rate at which the coprocessor 0 counter increments
  *
  * A MIPS CPU.
  */
@@ -1161,6 +1162,14 @@ struct MIPSCPU {
 
 CPUNegativeOffsetState neg;
 CPUMIPSState env;
+/*
+ * The Count register acts as a timer, incrementing at a constant rate,
+ * whether or not an instruction is executed, retired, or any forward
+ * progress is made through the pipeline. The rate at which the counter
+ * increments is implementation dependent, and is a function of the
+ * pipeline clock of the processor, not the issue width of the processor.
+ */
+unsigned cp0_count_rate;
 };
 
 
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 46188139b7b..461edfe22b7 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -26,7 +26,7 @@
 #include "qemu/module.h"
 #include "sysemu/kvm.h"
 #include "exec/exec-all.h"
-
+#include "hw/qdev-properties.h"
 
 static void mips_cpu_set_pc(CPUState *cs, vaddr value)
 {
@@ -135,12 +135,7 @@ static void mips_cpu_disas_set_info(CPUState *s, 
disassemble_info *info)
 }
 
 /*
- * Since commit 6af0bf9c7c3 this model assumes a CPU clocked at 200MHz
- * and a CP0 timer running at half the clock of the CPU (cp0_count_rate = 2).
- *
- * TIMER_FREQ_HZ = CPU_FREQ_HZ / CP0_COUNT_RATE = 200 MHz / 2 = 100 MHz
- *
- * TIMER_PERIOD_NS = 1 / TIMER_FREQ_HZ = 10 ns
+ * Since commit 6af0bf9c7c3 this model assumes a CPU clocked at 200MHz.
  */
 #define CPU_FREQ_HZ_DEFAULT 2
 #define CP0_COUNT_RATE_DEFAULT  2
@@ -149,7 +144,7 @@ static void mips_cp0_period_set(MIPSCPU *cpu)
 {
 CPUMIPSState *env = &cpu->env;
 
-env->cp0_count_ns = muldiv64(NANOSECONDS_PER_SECOND, 
CP0_COUNT_RATE_DEFAULT,
+env->cp0_count_ns = muldiv64(NANOSECONDS_PER_SECOND, cpu->cp0_count_rate,
  CPU_FREQ_HZ_DEFAULT);
 }
 
@@ -202,6 +197,13 @@ static ObjectClass *mips_cpu_class_by_name(const char 
*cpu_model)
 return oc;
 }
 
+static Property mips_cpu_properties[] = {
+/* CP0 timer running at half the clock of the CPU */
+DEFINE_PROP_UINT32("cp0-count-rate", MIPSCPU, cp0_count_rate,
+   CP0_COUNT_RATE_DEFAULT),
+DEFINE_PROP_END_OF_LIST()
+};
+
 static void mips_cpu_class_init(ObjectClass *c, void *data)
 {
 MIPSCPUClass *mcc = MIPS_CPU_CLASS(c);
@@ -211,6 +213,7 @@ static void mips_cpu_class_init(ObjectClass *c, void *data)
 device_class_set_parent_realize(dc, mips_cpu_realizefn,
 &mcc->parent_realize);
 device_class_set_parent_reset(dc, mips_cpu_reset, &mcc->parent_reset);
+device_class_set_props(dc, mips_cpu_properties);
 
 cc->class_by_name = mips_cpu_class_by_name;
 cc->has_work = mips_cpu_has_work;
-- 
2.26.2

[PATCH v4 08/21] target/mips: Move cp0_count_ns to CPUMIPSState

[Philippe Mathieu-Daudé]

Currently the CP0 timer period is fixed at 10 ns, corresponding
to a fixed CPU frequency of 200 MHz (using half the speed of the
CPU).

In few commits we will be able to use a different CPU frequency.
In preparation, move the cp0_count_ns variable to CPUMIPSState
so we can modify it.

Reviewed-by: Jiaxun Yang 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/mips/cpu.h   |  1 +
 target/mips/cp0_timer.c | 23 ++-
 target/mips/cpu.c   | 21 +
 3 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 7cf7f5239f7..085a88e9550 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -1145,6 +1145,7 @@ struct CPUMIPSState {
 struct MIPSITUState *itu;
 MemoryRegion *itc_tag; /* ITC Configuration Tags */
 target_ulong exception_base; /* ExceptionBase input to the core */
+uint64_t cp0_count_ns; /* CP0_Count clock period (in nanoseconds) */
 };
 
 /**
diff --git a/target/mips/cp0_timer.c b/target/mips/cp0_timer.c
index 6fec5fe0ff7..5ec0d6249e9 100644
--- a/target/mips/cp0_timer.c
+++ b/target/mips/cp0_timer.c
@@ -27,18 +27,6 @@
 #include "sysemu/kvm.h"
 #include "internal.h"
 
-/*
- * Since commit 6af0bf9c7c3 this model assumes a CPU clocked at 200MHz
- * and a CP0 timer running at half the clock of the CPU (cp0_count_rate = 2).
- *
- * TIMER_FREQ_HZ = CPU_FREQ_HZ / CP0_COUNT_RATE = 200 MHz / 2 = 100 MHz
- *
- * TIMER_PERIOD_NS = 1 / TIMER_FREQ_HZ = 10 ns
- */
-#define CPU_FREQ_HZ_DEFAULT 2
-#define CP0_COUNT_RATE_DEFAULT  2
-#define TIMER_PERIOD10  /* 1 / (CPU_FREQ_HZ / CP0_COUNT_RATE) */
-
 /* MIPS R4K timer */
 static void cpu_mips_timer_update(CPUMIPSState *env)
 {
@@ -47,8 +35,8 @@ static void cpu_mips_timer_update(CPUMIPSState *env)
 
 now_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 wait = env->CP0_Compare - env->CP0_Count -
-   (uint32_t)(now_ns / TIMER_PERIOD);
-next_ns = now_ns + (uint64_t)wait * TIMER_PERIOD;
+   (uint32_t)(now_ns / env->cp0_count_ns);
+next_ns = now_ns + (uint64_t)wait * env->cp0_count_ns;
 timer_mod(env->timer, next_ns);
 }
 
@@ -76,7 +64,7 @@ uint32_t cpu_mips_get_count(CPUMIPSState *env)
 cpu_mips_timer_expire(env);
 }
 
-return env->CP0_Count + (uint32_t)(now_ns / TIMER_PERIOD);
+return env->CP0_Count + (uint32_t)(now_ns / env->cp0_count_ns);
 }
 }
 
@@ -92,7 +80,8 @@ void cpu_mips_store_count(CPUMIPSState *env, uint32_t count)
 } else {
 /* Store new count register */
 env->CP0_Count = count -
-   (uint32_t)(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) / 
TIMER_PERIOD);
+   (uint32_t)(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) /
+  env->cp0_count_ns);
 /* Update timer timer */
 cpu_mips_timer_update(env);
 }
@@ -119,7 +108,7 @@ void cpu_mips_stop_count(CPUMIPSState *env)
 {
 /* Store the current value */
 env->CP0_Count += (uint32_t)(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) /
- TIMER_PERIOD);
+ env->cp0_count_ns);
 }
 
 static void mips_timer_cb(void *opaque)
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index e86cd065483..84b727fefa8 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -134,6 +134,25 @@ static void mips_cpu_disas_set_info(CPUState *s, 
disassemble_info *info)
 }
 }
 
+/*
+ * Since commit 6af0bf9c7c3 this model assumes a CPU clocked at 200MHz
+ * and a CP0 timer running at half the clock of the CPU (cp0_count_rate = 2).
+ *
+ * TIMER_FREQ_HZ = CPU_FREQ_HZ / CP0_COUNT_RATE = 200 MHz / 2 = 100 MHz
+ *
+ * TIMER_PERIOD_NS = 1 / TIMER_FREQ_HZ = 10 ns
+ */
+#define CPU_FREQ_HZ_DEFAULT 2
+#define CP0_COUNT_RATE_DEFAULT  2
+#define TIMER_PERIOD_DEFAULT10  /* 1 / (CPU_FREQ_HZ / CP0_COUNT_RATE) */
+
+static void mips_cp0_period_set(MIPSCPU *cpu)
+{
+CPUMIPSState *env = &cpu->env;
+
+env->cp0_count_ns = TIMER_PERIOD_DEFAULT;
+}
+
 static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
 {
 CPUState *cs = CPU(dev);
@@ -141,6 +160,8 @@ static void mips_cpu_realizefn(DeviceState *dev, Error 
**errp)
 MIPSCPUClass *mcc = MIPS_CPU_GET_CLASS(dev);
 Error *local_err = NULL;
 
+mips_cp0_period_set(cpu);
+
 cpu_exec_realizefn(cs, &local_err);
 if (local_err != NULL) {
 error_propagate(errp, local_err);
-- 
2.26.2

Re: [PATCH 1/3] block: push error reporting into bdrv_all_*_snapshot functions

[Max Reitz]

On 08.10.20 19:48, Philippe Mathieu-Daudé wrote:
> From: Daniel P. Berrangé 
> 
> The bdrv_all_*_snapshot functions return a BlockDriverState pointer
> for the invalid backend, which the callers then use to report an
> error message. In some cases multiple callers are reporting the
> same error message, but with slightly different text. In the future
> there will be more error scenarios for some of these methods, which
> will benefit from fine grained error message reporting. So it is
> helpful to push error reporting down a level.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  include/block/snapshot.h   | 14 +++
>  block/monitor/block-hmp-cmds.c |  7 ++--
>  block/snapshot.c   | 77 +-
>  migration/savevm.c | 37 +---
>  monitor/hmp-cmds.c |  7 +---
>  replay/replay-debugging.c  |  4 +-
>  tests/qemu-iotests/267.out | 10 ++---
>  7 files changed, 67 insertions(+), 89 deletions(-)

Looks good overall to me, but for some reason this patch pulls in the
@ok and @ret variables from the top scope of all concerned functions
into the inner scopes of the BDS loops, and drops their initialization.
 That’s wrong, because we only call the respective snapshotting
functions on some BDSs, so the return value stays uninitialized for all
other BDSs:

> diff --git a/block/snapshot.c b/block/snapshot.c
> index a2bf3a54eb..50e35bb9fa 100644
> --- a/block/snapshot.c
> +++ b/block/snapshot.c
> @@ -462,14 +462,14 @@ static bool 
> bdrv_all_snapshots_includes_bs(BlockDriverState *bs)
>   * These functions will properly handle dataplane (take aio_context_acquire
>   * when appropriate for appropriate block drivers) */
>  
> -bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs)
> +bool bdrv_all_can_snapshot(Error **errp)
>  {
> -bool ok = true;
>  BlockDriverState *bs;
>  BdrvNextIterator it;
>  
>  for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
>  AioContext *ctx = bdrv_get_aio_context(bs);
> +bool ok;

So I think @ok must be initialized.

>  
>  aio_context_acquire(ctx);
>  if (bdrv_all_snapshots_includes_bs(bs)) {
> @@ -477,26 +477,25 @@ bool bdrv_all_can_snapshot(BlockDriverState 
> **first_bad_bs)
>  }
>  aio_context_release(ctx);
>  if (!ok) {
> +error_setg(errp, "Device '%s' is writable but does not support "
> +   "snapshots", bdrv_get_device_or_node_name(bs));
>  bdrv_next_cleanup(&it);
> -goto fail;
> +return false;
>  }
>  }
>  
> -fail:
> -*first_bad_bs = bs;
> -return ok;
> +return true;
>  }
>  
> -int bdrv_all_delete_snapshot(const char *name, BlockDriverState 
> **first_bad_bs,
> - Error **errp)
> +int bdrv_all_delete_snapshot(const char *name, Error **errp)
>  {
> -int ret = 0;
>  BlockDriverState *bs;
>  BdrvNextIterator it;
>  QEMUSnapshotInfo sn1, *snapshot = &sn1;
>  
>  for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
>  AioContext *ctx = bdrv_get_aio_context(bs);
> +int ret;

Same here, @ret must be initialized.

>  
>  aio_context_acquire(ctx);
>  if (bdrv_all_snapshots_includes_bs(bs) &&
> @@ -507,26 +506,25 @@ int bdrv_all_delete_snapshot(const char *name, 
> BlockDriverState **first_bad_bs,
>  }
>  aio_context_release(ctx);
>  if (ret < 0) {
> +error_prepend(errp, "Could not delete snapshot '%s' on '%s': ",
> +  name, bdrv_get_device_or_node_name(bs));
>  bdrv_next_cleanup(&it);
> -goto fail;
> +return -1;
>  }
>  }
>  
> -fail:
> -*first_bad_bs = bs;
> -return ret;
> +return 0;
>  }
>  
>  
> -int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs,
> -   Error **errp)
> +int bdrv_all_goto_snapshot(const char *name, Error **errp)
>  {
> -int ret = 0;
>  BlockDriverState *bs;
>  BdrvNextIterator it;
>  
>  for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
>  AioContext *ctx = bdrv_get_aio_context(bs);
> +int ret;

And again.

>  
>  aio_context_acquire(ctx);
>  if (bdrv_all_snapshots_includes_bs(bs)) {
> @@ -534,75 +532,75 @@ int bdrv_all_goto_snapshot(const char *name, 
> BlockDriverState **first_bad_bs,
>  }
>  aio_context_release(ctx);
>  if (ret < 0) {
> +error_prepend(errp, "Could not load snapshot '%s' on '%s': ",
> +  name, bdrv_get_device_or_node_name(bs));
>  bdrv_next_cleanup(&it);
> -goto fail;
> +return -1;
>  }
>  }
>  
> -fail:
> -*first_bad_bs = bs;
> -return ret;
> +return 0;
>  }
>  
> -int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
> +int bdrv_all_find_snapshot(const char *

[PATCH v4 13/21] hw/mips/r4k: Explicit CPU frequency is 200 MHz

[Philippe Mathieu-Daudé]

Since its introduction in commit 6af0bf9c7c3,
the 'r4k' machine runs at 200 MHz.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/mips/r4k.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/mips/r4k.c b/hw/mips/r4k.c
index 3487013a4a1..39bc626e7c5 100644
--- a/hw/mips/r4k.c
+++ b/hw/mips/r4k.c
@@ -13,6 +13,7 @@
 #include "qapi/error.h"
 #include "qemu-common.h"
 #include "cpu.h"
+#include "hw/clock.h"
 #include "hw/mips/mips.h"
 #include "hw/mips/cpudevs.h"
 #include "hw/intc/i8259.h"
@@ -182,6 +183,7 @@ void mips_r4k_init(MachineState *machine)
 MemoryRegion *isa_io = g_new(MemoryRegion, 1);
 MemoryRegion *isa_mem = g_new(MemoryRegion, 1);
 int bios_size;
+Clock *cpuclk;
 MIPSCPU *cpu;
 CPUMIPSState *env;
 ResetData *reset_info;
@@ -192,8 +194,11 @@ void mips_r4k_init(MachineState *machine)
 DriveInfo *dinfo;
 int be;
 
+cpuclk = clock_new(OBJECT(machine), "cpu-refclk");
+clock_set_hz(cpuclk, 2); /* 200 MHz */
+
 /* init CPUs */
-cpu = MIPS_CPU(cpu_create(machine->cpu_type));
+cpu = mips_cpu_create_with_clock(machine->cpu_type, cpuclk);
 env = &cpu->env;
 
 reset_info = g_malloc0(sizeof(ResetData));
-- 
2.26.2

[PATCH v4 18/21] hw/mips/boston: Set CPU frequency to 1 GHz

[Philippe Mathieu-Daudé]

The I6400 can run at 1 GHz or more. Create a 'cpuclk'
output clock and connect it to the CPU input clock.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/mips/boston.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index 1b3f69e949c..cf2296f4488 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -30,6 +30,7 @@
 #include "hw/mips/cps.h"
 #include "hw/mips/cpudevs.h"
 #include "hw/pci-host/xilinx-pcie.h"
+#include "hw/qdev-clock.h"
 #include "hw/qdev-properties.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
@@ -54,6 +55,7 @@ struct BostonState {
 MachineState *mach;
 MIPSCPSState cps;
 SerialMM *uart;
+Clock *cpuclk;
 
 CharBackend lcd_display;
 char lcd_content[8];
@@ -251,10 +253,19 @@ static const MemoryRegionOps boston_platreg_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static void mips_boston_instance_init(Object *obj)
+{
+BostonState *s = BOSTON(obj);
+
+s->cpuclk = qdev_init_clock_out(DEVICE(obj), "cpu-refclk");
+clock_set_hz(s->cpuclk, 10); /* 1 GHz */
+}
+
 static const TypeInfo boston_device = {
 .name  = TYPE_MIPS_BOSTON,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(BostonState),
+.instance_init = mips_boston_instance_init,
 };
 
 static void boston_register_types(void)
@@ -462,6 +473,8 @@ static void boston_mach_init(MachineState *machine)
 &error_fatal);
 object_property_set_int(OBJECT(&s->cps), "num-vp", machine->smp.cpus,
 &error_fatal);
+qdev_connect_clock_in(DEVICE(&s->cps), "clk-in",
+  qdev_get_clock_out(dev, "cpu-refclk"));
 sysbus_realize(SYS_BUS_DEVICE(&s->cps), &error_fatal);
 
 sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1);
-- 
2.26.2

[PATCH v4 12/21] target/mips/cpu: Introduce mips_cpu_create_with_clock() helper

[Philippe Mathieu-Daudé]

Introduce an helper to create a MIPS CPU and connect it to
a reference clock. This helper is not MIPS specific, but so
far only MIPS CPUs need it.

Suggested-by: Huacai Chen 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/mips/cpu.h | 12 
 target/mips/cpu.c | 12 
 2 files changed, 24 insertions(+)

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 062a4ba6225..d41579d44ae 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -1307,4 +1307,16 @@ static inline void cpu_get_tb_cpu_state(CPUMIPSState 
*env, target_ulong *pc,
 MIPS_HFLAG_HWRENA_ULR);
 }
 
+/**
+ * mips_cpu_create_with_clock:
+ * @typename: a MIPS CPU type.
+ * @cpu_refclk: this cpu input clock (an output clock of another device)
+ *
+ * Instantiates a MIPS CPU, set the input clock of the CPU to @cpu_refclk,
+ * then realizes the CPU.
+ *
+ * Returns: A #CPUState or %NULL if an error occurred.
+ */
+MIPSCPU *mips_cpu_create_with_clock(const char *cpu_type, Clock *cpu_refclk);
+
 #endif /* MIPS_CPU_H */
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 3deb0245e7c..23f7b4508a4 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -287,3 +287,15 @@ static void mips_cpu_register_types(void)
 }
 
 type_init(mips_cpu_register_types)
+
+/* Could be used by generic CPU object */
+MIPSCPU *mips_cpu_create_with_clock(const char *cpu_type, Clock *cpu_refclk)
+{
+DeviceState *cpu;
+
+cpu = DEVICE(object_new(cpu_type));
+qdev_connect_clock_in(cpu, "clk-in", cpu_refclk);
+qdev_realize(cpu, NULL, &error_abort);
+
+return MIPS_CPU(cpu);
+}
-- 
2.26.2

[PATCH v4 20/21] hw/mips/cps: Do not allow use without input clock

[Philippe Mathieu-Daudé]

Now than all QOM users provides the input clock, do not allow
using a CPS without input clock connected.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/mips/cps.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/mips/cps.c b/hw/mips/cps.c
index af7b58c4bdd..c624821315a 100644
--- a/hw/mips/cps.c
+++ b/hw/mips/cps.c
@@ -74,6 +74,11 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
 bool itu_present = false;
 bool saar_present = false;
 
+if (!clock_get(s->clock)) {
+error_setg(errp, "CPS input clock is not connected to an output 
clock");
+return;
+}
+
 for (i = 0; i < s->num_vp; i++) {
 cpu = MIPS_CPU(object_new(s->cpu_type));
 
-- 
2.26.2

[PATCH v4 15/21] hw/mips/mipssim: Correct CPU frequency

[Philippe Mathieu-Daudé]

The MIPSsim machine CPU frequency is too fast running at 200 MHz,
while it should be 12 MHz for the 24K and 6 MHz for the 5K core.

Ref: Linux commit c78cbf49c4ed
("Support for MIPSsim, the cycle accurate MIPS simulator.")

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/mips/mipssim.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/mips/mipssim.c b/hw/mips/mipssim.c
index 5d4ad74828d..f0042f7f436 100644
--- a/hw/mips/mipssim.c
+++ b/hw/mips/mipssim.c
@@ -29,6 +29,7 @@
 #include "qapi/error.h"
 #include "qemu-common.h"
 #include "cpu.h"
+#include "hw/clock.h"
 #include "hw/mips/mips.h"
 #include "hw/mips/cpudevs.h"
 #include "hw/char/serial.h"
@@ -150,13 +151,21 @@ mips_mipssim_init(MachineState *machine)
 MemoryRegion *address_space_mem = get_system_memory();
 MemoryRegion *isa = g_new(MemoryRegion, 1);
 MemoryRegion *bios = g_new(MemoryRegion, 1);
+Clock *cpuclk;
 MIPSCPU *cpu;
 CPUMIPSState *env;
 ResetData *reset_info;
 int bios_size;
 
+cpuclk = clock_new(OBJECT(machine), "cpu-refclk");
+#ifdef TARGET_MIPS64
+clock_set_hz(cpuclk, 600); /* 6 MHz */
+#else
+clock_set_hz(cpuclk, 1200); /* 12 MHz */
+#endif
+
 /* Init CPUs. */
-cpu = MIPS_CPU(cpu_create(machine->cpu_type));
+cpu = mips_cpu_create_with_clock(machine->cpu_type, cpuclk);
 env = &cpu->env;
 
 reset_info = g_malloc0(sizeof(ResetData));
-- 
2.26.2

[PATCH v4 14/21] hw/mips/fuloong2e: Set CPU frequency to 533 MHz

[Philippe Mathieu-Daudé]

The CPU frequency is normally provided by the firmware in the
"cpuclock" environment variable. The 2E board can handles up
to 660MHz, but be conservative and take the same value used
by the Linux kernel: 533 MHz.

Reviewed-by: Jiaxun Yang 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/mips/fuloong2e.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index b000ed1d7f7..b8234f61083 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -23,6 +23,7 @@
 #include "qemu/units.h"
 #include "qapi/error.h"
 #include "cpu.h"
+#include "hw/clock.h"
 #include "hw/intc/i8259.h"
 #include "hw/dma/i8257.h"
 #include "hw/isa/superio.h"
@@ -298,12 +299,16 @@ static void mips_fuloong2e_init(MachineState *machine)
 PCIBus *pci_bus;
 ISABus *isa_bus;
 I2CBus *smbus;
+Clock *cpuclk;
 MIPSCPU *cpu;
 CPUMIPSState *env;
 DeviceState *dev;
 
+cpuclk = clock_new(OBJECT(machine), "cpu-refclk");
+clock_set_hz(cpuclk, 53308); /* ~533 MHz */
+
 /* init CPUs */
-cpu = MIPS_CPU(cpu_create(machine->cpu_type));
+cpu = mips_cpu_create_with_clock(machine->cpu_type, cpuclk);
 env = &cpu->env;
 
 qemu_register_reset(main_cpu_reset, cpu);
-- 
2.26.2

[PATCH v4 19/21] hw/mips/malta: Set CPU frequency to 320 MHz

[Philippe Mathieu-Daudé]

The CoreLV card with ID 0x420's CPU clocked at 320 MHz. Create
a 'cpuclk' output clock and connect it to the CPU input clock.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/mips/malta.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 4019c9dc1a8..1e2b750719e 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -26,6 +26,7 @@
 #include "qemu/units.h"
 #include "qemu-common.h"
 #include "cpu.h"
+#include "hw/clock.h"
 #include "hw/southbridge/piix.h"
 #include "hw/isa/superio.h"
 #include "hw/char/serial.h"
@@ -57,6 +58,7 @@
 #include "sysemu/kvm.h"
 #include "hw/semihosting/semihost.h"
 #include "hw/mips/cps.h"
+#include "hw/qdev-clock.h"
 
 #define ENVP_ADDR   0x80002000l
 #define ENVP_NB_ENTRIES 16
@@ -94,6 +96,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(MaltaState, MIPS_MALTA)
 struct MaltaState {
 SysBusDevice parent_obj;
 
+Clock *cpuclk;
 MIPSCPSState cps;
 qemu_irq i8259[ISA_NUM_IRQS];
 };
@@ -1159,7 +1162,7 @@ static void main_cpu_reset(void *opaque)
 }
 }
 
-static void create_cpu_without_cps(MachineState *ms,
+static void create_cpu_without_cps(MachineState *ms, MaltaState *s,
qemu_irq *cbus_irq, qemu_irq *i8259_irq)
 {
 CPUMIPSState *env;
@@ -1167,7 +1170,7 @@ static void create_cpu_without_cps(MachineState *ms,
 int i;
 
 for (i = 0; i < ms->smp.cpus; i++) {
-cpu = MIPS_CPU(cpu_create(ms->cpu_type));
+cpu = mips_cpu_create_with_clock(ms->cpu_type, s->cpuclk);
 
 /* Init internal devices */
 cpu_mips_irq_init_cpu(cpu);
@@ -1189,6 +1192,7 @@ static void create_cps(MachineState *ms, MaltaState *s,
 &error_fatal);
 object_property_set_int(OBJECT(&s->cps), "num-vp", ms->smp.cpus,
 &error_fatal);
+qdev_connect_clock_in(DEVICE(&s->cps), "clk-in", s->cpuclk);
 sysbus_realize(SYS_BUS_DEVICE(&s->cps), &error_fatal);
 
 sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1);
@@ -1203,7 +1207,7 @@ static void mips_create_cpu(MachineState *ms, MaltaState 
*s,
 if ((ms->smp.cpus > 1) && cpu_supports_cps_smp(ms->cpu_type)) {
 create_cps(ms, s, cbus_irq, i8259_irq);
 } else {
-create_cpu_without_cps(ms, cbus_irq, i8259_irq);
+create_cpu_without_cps(ms, s, cbus_irq, i8259_irq);
 }
 }
 
@@ -1421,10 +1425,19 @@ void mips_malta_init(MachineState *machine)
 pci_vga_init(pci_bus);
 }
 
+static void mips_malta_instance_init(Object *obj)
+{
+MaltaState *s = MIPS_MALTA(obj);
+
+s->cpuclk = qdev_init_clock_out(DEVICE(obj), "cpu-refclk");
+clock_set_hz(s->cpuclk, 32000); /* 320 MHz */
+}
+
 static const TypeInfo mips_malta_device = {
 .name  = TYPE_MIPS_MALTA,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(MaltaState),
+.instance_init = mips_malta_instance_init,
 };
 
 static void mips_malta_machine_init(MachineClass *mc)
-- 
2.26.2

[PATCH v4 21/21] target/mips/cpu: Display warning when CPU is used without input clock

[Philippe Mathieu-Daudé]

All our QOM users provides an input clock. In order to avoid
avoid future machines added without clock, display a warning.

User-mode emulation use the CP0 timer with the RDHWR instruction
(see commit cdfcad788394) so keep using the fixed 200 MHz clock
without diplaying any warning. Only display it in system-mode
emulation.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/mips/cpu.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 23f7b4508a4..8daa5878ba1 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -19,12 +19,14 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "cpu.h"
 #include "internal.h"
 #include "kvm_mips.h"
 #include "qemu/module.h"
 #include "sysemu/kvm.h"
+#include "sysemu/qtest.h"
 #include "exec/exec-all.h"
 #include "hw/qdev-properties.h"
 #include "hw/qdev-clock.h"
@@ -157,6 +159,14 @@ static void mips_cpu_realizefn(DeviceState *dev, Error 
**errp)
 Error *local_err = NULL;
 
 if (!clock_get(cpu->clock)) {
+#ifndef CONFIG_USER_ONLY
+if (!qtest_enabled()) {
+g_autofree char *cpu_freq_str = freq_to_str(CPU_FREQ_HZ_DEFAULT);
+
+warn_report("CPU input clock is not connected to any output clock, 
"
+"using default frequency of %s.", cpu_freq_str);
+}
+#endif
 /* Initialize the frequency in case the clock remains unconnected. */
 clock_set_hz(cpu->clock, CPU_FREQ_HZ_DEFAULT);
 }
-- 
2.26.2

[PATCH v4 16/21] hw/mips/jazz: Correct CPU frequencies

[Philippe Mathieu-Daudé]

The Magnum 4000PC CPU runs at 100 MHz, and the Acer PICA-61
CPU at ~134 MHz.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/mips/jazz.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
index 47723093b63..8f1ad55ba34 100644
--- a/hw/mips/jazz.c
+++ b/hw/mips/jazz.c
@@ -24,6 +24,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu-common.h"
+#include "hw/clock.h"
 #include "hw/mips/mips.h"
 #include "hw/mips/cpudevs.h"
 #include "hw/intc/i8259.h"
@@ -142,6 +143,7 @@ static void mips_jazz_init(MachineState *machine,
 MemoryRegion *address_space = get_system_memory();
 char *filename;
 int bios_size, n;
+Clock *cpuclk;
 MIPSCPU *cpu;
 CPUClass *cc;
 CPUMIPSState *env;
@@ -163,14 +165,25 @@ static void mips_jazz_init(MachineState *machine,
 MemoryRegion *bios2 = g_new(MemoryRegion, 1);
 SysBusESPState *sysbus_esp;
 ESPState *esp;
+static const struct {
+unsigned freq_hz;
+unsigned pll_mult;
+} ext_clk[] = {
+[JAZZ_MAGNUM] = {5000, 2},
+[JAZZ_PICA61] = {, 4},
+};
 
 if (machine->ram_size > 256 * MiB) {
 error_report("RAM size more than 256Mb is not supported");
 exit(EXIT_FAILURE);
 }
 
+cpuclk = clock_new(OBJECT(machine), "cpu-refclk");
+clock_set_hz(cpuclk, ext_clk[jazz_model].freq_hz
+ * ext_clk[jazz_model].pll_mult);
+
 /* init CPUs */
-cpu = MIPS_CPU(cpu_create(machine->cpu_type));
+cpu = mips_cpu_create_with_clock(machine->cpu_type, cpuclk);
 env = &cpu->env;
 qemu_register_reset(main_cpu_reset, cpu);
 
-- 
2.26.2

Re: [PATCH] gitlab-ci.yml: Only run one test-case per fuzzer

[Thomas Huth]

On 02/10/2020 16.35, Alexander Bulekov wrote:
> With 1000 runs, there is a non-negligible chance that the fuzzer can
> trigger a crash. With this CI job, we care about catching build/runtime
> issues in the core fuzzing code. Actual device fuzzing takes place on
> oss-fuzz. For these purposes, only running one input should be
> sufficient.
> 
> Signed-off-by: Alexander Bulekov 
> Suggested-by: Philippe Mathieu-Daudé 
> ---
>  .gitlab-ci.yml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index a51c89554f..075c15d45c 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -303,7 +303,7 @@ build-oss-fuzz:
>| grep -v slirp); do
>  grep "LLVMFuzzerTestOneInput" ${fuzzer} > /dev/null 2>&1 || continue 
> ;
>  echo Testing ${fuzzer} ... ;
> -"${fuzzer}" -runs=1000 -seed=1 || exit 1 ;
> +"${fuzzer}" -runs=1 -seed=1 || exit 1 ;
>done
>  # Unrelated to fuzzer: run some tests with -fsanitize=address
>  - cd build-oss-fuzz && make check-qtest-i386 check-unit

Thanks, queued to:

 https://gitlab.com/huth/qemu/-/commits/qtest-next/

 Thomas

[PATCH v4 17/21] hw/mips/cps: Expose input clock and connect it to CPU cores

[Philippe Mathieu-Daudé]

Expose a qdev input clock named 'clk-in', and connect it to each
core to forward-propagate the clock.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/mips/cps.h | 2 ++
 hw/mips/cps.c | 4 
 2 files changed, 6 insertions(+)

diff --git a/include/hw/mips/cps.h b/include/hw/mips/cps.h
index 9e35a881366..859a8d4a674 100644
--- a/include/hw/mips/cps.h
+++ b/include/hw/mips/cps.h
@@ -21,6 +21,7 @@
 #define MIPS_CPS_H
 
 #include "hw/sysbus.h"
+#include "hw/clock.h"
 #include "hw/misc/mips_cmgcr.h"
 #include "hw/intc/mips_gic.h"
 #include "hw/misc/mips_cpc.h"
@@ -43,6 +44,7 @@ struct MIPSCPSState {
 MIPSGICState gic;
 MIPSCPCState cpc;
 MIPSITUState itu;
+Clock *clock;
 };
 
 qemu_irq get_cps_irq(MIPSCPSState *cps, int pin_number);
diff --git a/hw/mips/cps.c b/hw/mips/cps.c
index 23c0f87e41a..af7b58c4bdd 100644
--- a/hw/mips/cps.c
+++ b/hw/mips/cps.c
@@ -22,6 +22,7 @@
 #include "qemu/module.h"
 #include "hw/mips/cps.h"
 #include "hw/mips/mips.h"
+#include "hw/qdev-clock.h"
 #include "hw/qdev-properties.h"
 #include "hw/mips/cpudevs.h"
 #include "sysemu/kvm.h"
@@ -38,6 +39,7 @@ static void mips_cps_init(Object *obj)
 SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 MIPSCPSState *s = MIPS_CPS(obj);
 
+s->clock = qdev_init_clock_in(DEVICE(obj), "clk-in", NULL, NULL);
 /*
  * Cover entire address space as there do not seem to be any
  * constraints for the base address of CPC and GIC.
@@ -80,6 +82,8 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
   errp)) {
 return;
 }
+/* All cores use the same clock tree */
+qdev_connect_clock_in(DEVICE(cpu), "clk-in", s->clock);
 
 if (!qdev_realize_and_unref(DEVICE(cpu), NULL, errp)) {
 return;
-- 
2.26.2

Re: [PATCH 1/3] hw/ssi/aspeed_smc: Rename max_slaves as max_devices

[Kevin Wolf]

Am 11.10.2020 um 23:05 hat Philippe Mathieu-Daudé geschrieben:
> From: Philippe Mathieu-Daudé 
> 
> In order to use inclusive terminology, rename max_slaves
> as max_peripherals.

This is inconsistent with the subject line which talks about
"max_devices".

Kevin

Re: [RFC v1 3/4] qtest: do not build ide-test if TCG is not available

[Claudio Fontana]

On 10/10/20 12:50 PM, Claudio Fontana wrote:
> On 10/9/20 6:01 PM, Paolo Bonzini wrote:
>> On 09/10/20 17:21, Claudio Fontana wrote:
>>> it seems that ide-test depends on TCG currently.
>>>
>>> Signed-off-by: Claudio Fontana 
>>> ---
>>>  tests/qtest/meson.build | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
>>> index ad33ac311d..3418f65e2a 100644
>>> --- a/tests/qtest/meson.build
>>> +++ b/tests/qtest/meson.build
>>> @@ -46,9 +46,9 @@ qtests_i386 = \
>>>(config_all_devices.has_key('CONFIG_TPM_TIS_ISA') ? ['tpm-tis-test'] : 
>>> []) +  \
>>>(config_all_devices.has_key('CONFIG_TPM_TIS_ISA') ? 
>>> ['tpm-tis-swtpm-test'] : []) +\
>>>(config_all_devices.has_key('CONFIG_RTL8139_PCI') ? ['rtl8139-test'] : 
>>> []) +  \
>>> +  (config_all.has_key('CONFIG_TCG') ? ['ide-test'] : []) + 
>>>  \
>>>qtests_pci + 
>>>  \
>>>['fdc-test',
>>> -   'ide-test',
>>> 'hd-geo-test',
>>> 'boot-order-test',
>>> 'bios-tables-test',
>>>
>>
>> Interesting, why?...
>>
>> Paolo
>>
>>
> 
> I am slowly trying to find out. I found out that the qos-test that buzzes is 
> ide-test,
> and I found out which specific ide test it was by manually bisecting 
> functions inside the qtest_add_func in ide-test.c.
> 
> The issue seems limited to qtest_add_func("/ide/bmdma/trim", test_bmdma_trim);
> No idea yet why that test buzzes forever.
> 
> Side note, maybe more verbose output on which specific test is attempted 
> could be helpful? maybe only enabled on make V=2 ?
> 
> So the buzz.
> top says:
> 
> 22621 claudio   20   0   89700   3292   3004 R 53.82 0.010   1:22.43 ide-test 
>  
> 22844 claudio   20   0 1026700  61168  38632 R 99.67 0.188   2:39.53 
> qemu-system-i38   
> 25325 claudio   20   0   89700   3208   2940 R 52.16 0.010   0:56.05 ide-test 
>   
> 25403 claudio   20   0 1026720  63028  38416 R 99.67 0.194   1:48.63 
> qemu-system-x86   
> 
> 
> i386 and x86_64 seem to show the exact same behaviour.
> 
> 
> gdb says:
> 
> qemu-system-x86 (25403):
> 
> (gdb) info threads
>   Id   Target Id   Frame 
> * 1Thread 0x7fe35a406140 (LWP 25403) "qemu-system-x86" 0x7fe35157f7d6 
> in ppoll () from /lib64/libc.so.6
>   2Thread 0x7fe33946e700 (LWP 25415) "qemu-system-x86" 0x7fe351584839 
> in syscall () from /lib64/libc.so.6
>   3Thread 0x7fe338c6d700 (LWP 25439) "qemu-system-x86" 0x7fe35157f6db 
> in poll () from /lib64/libc.so.6
>   4Thread 0x7fe333fff700 (LWP 25440) "qemu-system-x86" 0x7fe35185bdcf 
> in do_sigwait () from /lib64/libpthread.so.0
> 
> (gdb) thread 1
> [Switching to thread 1 (Thread 0x7fe35a406140 (LWP 25403))]
> #0  0x7fe35157f7d6 in ppoll () from /lib64/libc.so.6
> (gdb) bt
> #0  0x7fe35157f7d6 in ppoll () at /lib64/libc.so.6
> #1  0x55a1f3138309 in ppoll (__ss=0x0, __timeout=0x7fff64d10b70, 
> __nfds=, __fds=)
> at /usr/include/bits/poll2.h:77
> #2  0x55a1f3138309 in qemu_poll_ns (fds=, nfds= out>, timeout=timeout@entry=27462700)
> at ../util/qemu-timer.c:349
> #3  0x55a1f31512a5 in os_host_main_loop_wait (timeout=27462700) at 
> ../util/main-loop.c:239
> #4  0x55a1f31512a5 in main_loop_wait (nonblocking=nonblocking@entry=0) at 
> ../util/main-loop.c:520
> #5  0x55a1f2fc4bbd in qemu_main_loop () at ../softmmu/vl.c:1677
> #6  0x55a1f2d001fe in main (argc=, argv=, 
> envp=) at ../softmmu/main.c:50
> 
> (gdb) thread 2
> [Switching to thread 2 (Thread 0x7fe33946e700 (LWP 25415))]
> #0  0x7fe351584839 in syscall () from /lib64/libc.so.6
> (gdb) bt
> #0  0x7fe351584839 in syscall () at /lib64/libc.so.6
> #1  0x55a1f312605b in qemu_futex_wait (val=, f= out>)
> at /home/claudio/git/qemu-pristine/qemu/include/qemu/futex.h:29
> #2  0x55a1f312605b in qemu_event_wait (ev=ev@entry=0x55a1f3a44208 
> ) at ../util/qemu-thread-posix.c:460
> #3  0x55a1f314f868 in call_rcu_thread (opaque=opaque@entry=0x0) at 
> ../util/rcu.c:258
> #4  0x55a1f3125276 in qemu_thread_start (args=) at 
> ../util/qemu-thread-posix.c:521
> #5  0x7fe3518514f9 in start_thread () at /lib64/libpthread.so.0
> #6  0x7fe351589fbf in clone () at /lib64/libc.so.6
> (gdb) frame 3
> #3  0x55a1f314f868 in call_rcu_thread (opaque=opaque@entry=0x0) at 
> ../util/rcu.c:258
> 258 qemu_event_wait(&rcu_call_ready_event);
> (gdb) list 258
> 253 n = qatomic_read(&rcu_call_count);
> 254 if (n == 0) {
> 255 #if defined(CONFIG_MALLOC_TRIM)
> 256 malloc_trim(4 * 1024 * 1024);
> 257 #endif
> 258 qemu_event_wait

Re: [PATCH 1/3] block: push error reporting into bdrv_all_*_snapshot functions

[Philippe Mathieu-Daudé]

On 10/12/20 12:07 PM, Max Reitz wrote:

On 08.10.20 19:48, Philippe Mathieu-Daudé wrote:

From: Daniel P. Berrangé 

The bdrv_all_*_snapshot functions return a BlockDriverState pointer
for the invalid backend, which the callers then use to report an
error message. In some cases multiple callers are reporting the
same error message, but with slightly different text. In the future
there will be more error scenarios for some of these methods, which
will benefit from fine grained error message reporting. So it is
helpful to push error reporting down a level.

Signed-off-by: Daniel P. Berrangé 
---
  include/block/snapshot.h   | 14 +++
  block/monitor/block-hmp-cmds.c |  7 ++--
  block/snapshot.c   | 77 +-
  migration/savevm.c | 37 +---
  monitor/hmp-cmds.c |  7 +---
  replay/replay-debugging.c  |  4 +-
  tests/qemu-iotests/267.out | 10 ++---
  7 files changed, 67 insertions(+), 89 deletions(-)


Looks good overall to me, but for some reason this patch pulls in the
@ok and @ret variables from the top scope of all concerned functions
into the inner scopes of the BDS loops, and drops their initialization.
  That’s wrong, because we only call the respective snapshotting
functions on some BDSs, so the return value stays uninitialized for all
other BDSs:


Indeed, thanks for catching that.

[...]

  int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
   BlockDriverState *vm_state_bs,
   uint64_t vm_state_size,
- BlockDriverState **first_bad_bs)
+ Error **errp)
  {
-int err = 0;
  BlockDriverState *bs;
  BdrvNextIterator it;
  
  for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {

  AioContext *ctx = bdrv_get_aio_context(bs);
+int ret;


And one final time.

Max


  aio_context_acquire(ctx);
  if (bs == vm_state_bs) {
  sn->vm_state_size = vm_state_size;
-err = bdrv_snapshot_create(bs, sn);
+ret = bdrv_snapshot_create(bs, sn);
  } else if (bdrv_all_snapshots_includes_bs(bs)) {
  sn->vm_state_size = 0;
-err = bdrv_snapshot_create(bs, sn);
+ret = bdrv_snapshot_create(bs, sn);


This one is not needed.


  }
  aio_context_release(ctx);
-if (err < 0) {
+if (ret < 0) {
+error_setg(errp, "Could not create snapshot '%s' on '%s'",
+   sn->name, bdrv_get_device_or_node_name(bs));
  bdrv_next_cleanup(&it);
-goto fail;
+return -1;
  }
  }

[PATCH] spapr: Move spapr_create_nvdimm_dr_connectors() to core machine code

[Greg Kurz]

The spapr_create_nvdimm_dr_connectors() function doesn't need to access
any internal details of the sPAPR NVDIMM implementation. Also, pretty
much like for the LMBs, only spapr_machine_init() is responsible for the
creation of DR connectors for NVDIMMs.

Make this clear by making this function static in hw/ppc/spapr.c.

Signed-off-by: Greg Kurz 
---
 hw/ppc/spapr.c|   10 ++
 hw/ppc/spapr_nvdimm.c |   11 ---
 include/hw/ppc/spapr_nvdimm.h |1 -
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 63315f2d0fa9..ee716a12af73 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2641,6 +2641,16 @@ static hwaddr spapr_rma_size(SpaprMachineState *spapr, 
Error **errp)
 return rma_size;
 }
 
+static void spapr_create_nvdimm_dr_connectors(SpaprMachineState *spapr)
+{
+MachineState *machine = MACHINE(spapr);
+int i;
+
+for (i = 0; i < machine->ram_slots; i++) {
+spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_PMEM, i);
+}
+}
+
 /* pSeries LPAR / sPAPR hardware init */
 static void spapr_machine_init(MachineState *machine)
 {
diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
index b3a489e9fe18..9e3d94071fe1 100644
--- a/hw/ppc/spapr_nvdimm.c
+++ b/hw/ppc/spapr_nvdimm.c
@@ -106,17 +106,6 @@ void spapr_add_nvdimm(DeviceState *dev, uint64_t slot, 
Error **errp)
 }
 }
 
-void spapr_create_nvdimm_dr_connectors(SpaprMachineState *spapr)
-{
-MachineState *machine = MACHINE(spapr);
-int i;
-
-for (i = 0; i < machine->ram_slots; i++) {
-spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_PMEM, i);
-}
-}
-
-
 static int spapr_dt_nvdimm(SpaprMachineState *spapr, void *fdt,
int parent_offset, NVDIMMDevice *nvdimm)
 {
diff --git a/include/hw/ppc/spapr_nvdimm.h b/include/hw/ppc/spapr_nvdimm.h
index b834d82f5545..490b19a009f4 100644
--- a/include/hw/ppc/spapr_nvdimm.h
+++ b/include/hw/ppc/spapr_nvdimm.h
@@ -31,6 +31,5 @@ void spapr_dt_persistent_memory(SpaprMachineState *spapr, 
void *fdt);
 bool spapr_nvdimm_validate(HotplugHandler *hotplug_dev, NVDIMMDevice *nvdimm,
uint64_t size, Error **errp);
 void spapr_add_nvdimm(DeviceState *dev, uint64_t slot, Error **errp);
-void spapr_create_nvdimm_dr_connectors(SpaprMachineState *spapr);
 
 #endif

Re: [PATCH 1/3] hw/ssi/aspeed_smc: Rename max_slaves as max_devices

[Philippe Mathieu-Daudé]

On 10/12/20 12:04 PM, Kevin Wolf wrote:

Am 11.10.2020 um 23:05 hat Philippe Mathieu-Daudé geschrieben:

From: Philippe Mathieu-Daudé 

In order to use inclusive terminology, rename max_slaves
as max_peripherals.


This is inconsistent with the subject line which talks about
"max_devices".


Ah I missed that change, thanks.

Re: [PATCH] spapr: Move spapr_create_nvdimm_dr_connectors() to core machine code

[Philippe Mathieu-Daudé]

On 10/12/20 12:15 PM, Greg Kurz wrote:

The spapr_create_nvdimm_dr_connectors() function doesn't need to access
any internal details of the sPAPR NVDIMM implementation. Also, pretty
much like for the LMBs, only spapr_machine_init() is responsible for the
creation of DR connectors for NVDIMMs.

Make this clear by making this function static in hw/ppc/spapr.c.

Signed-off-by: Greg Kurz 
---
  hw/ppc/spapr.c|   10 ++
  hw/ppc/spapr_nvdimm.c |   11 ---
  include/hw/ppc/spapr_nvdimm.h |1 -
  3 files changed, 10 insertions(+), 12 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: Ping2: [PATCH v2] Emulate dip switch language layout settings on SUN keyboard

[Mark Cave-Ayland]

On 10/10/2020 15:44, Artyom Tarasenko wrote:

> Hi Mark,
> 
> could you please pick it up?
> 
> Regards,
> Artyom

Sure, I'll add it to my list. I'm currently starting to work through my 
backlog...


ATB,

Mark.

Re: [PATCH v2] qtest: add fuzz test case

[Thomas Huth]

On 21/09/2020 18.06, Li Qiang wrote:
> Currently the device fuzzer find a more and more issues.
> For every fuzz case, we need not only the fixes but also
> the corresponding test case. We can analysis the reproducer
> for every case and find what happened in where and write
> a beautiful test case. However the raw data of reproducer is not
> friendly to analysis. It will take a very long time, even far more
> than the fixes itself. So let's create a new file to hold all of
> the fuzz test cases and just use the raw data to act as the test
> case. This way nobody will be afraid of writing a test case for
> the fuzz reproducer.
> 
> This patch adds the issue LP#1878263 test case.
> 
> Signed-off-by: Li Qiang 
> ---
> Change since v1:
> rename the test function
> limit the test to i386/x86_64 arch
> using meson build system
> 
>  tests/qtest/fuzz-test.c | 51 +
>  tests/qtest/meson.build |  1 +
>  2 files changed, 52 insertions(+)
>  create mode 100644 tests/qtest/fuzz-test.c
> 
> diff --git a/tests/qtest/fuzz-test.c b/tests/qtest/fuzz-test.c
> new file mode 100644
> index 00..4398ccf137
> --- /dev/null
> +++ b/tests/qtest/fuzz-test.c
> @@ -0,0 +1,51 @@
> +/*
> + * QTest testcase for fuzz case
> + *
> + * Copyright (c) 2020 Li Qiang 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +
> +#include "qemu/osdep.h"
> +
> +#include "libqos/libqtest.h"
> +
> +/*
> + * This used to trigger the assert in scsi_dma_complete
> + * https://bugs.launchpad.net/qemu/+bug/1878263
> + */
> +static void test_lp1878263_megasas_zero_iov_cnt(void)
> +{
> +QTestState *s;
> +
> +s = qtest_init("-nographic -monitor none -serial none "
> +   "-M q35 -device megasas -device scsi-cd,drive=null0 "
> +   "-blockdev 
> driver=null-co,read-zeroes=on,node-name=null0");
> +qtest_outl(s, 0xcf8, 0x80001818);
> +qtest_outl(s, 0xcfc, 0xc101);
> +qtest_outl(s, 0xcf8, 0x8000181c);
> +qtest_outl(s, 0xcf8, 0x80001804);
> +qtest_outw(s, 0xcfc, 0x7);
> +qtest_outl(s, 0xcf8, 0x8000186a);
> +qtest_writeb(s, 0x14, 0xfe);
> +qtest_writeb(s, 0x0, 0x02);
> +qtest_outb(s, 0xc1c0, 0x17);
> +qtest_quit(s);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +const char *arch = qtest_get_arch();
> +
> +g_test_init(&argc, &argv, NULL);
> +
> +if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +qtest_add_func("fuzz/test_lp1878263_megasas_zero_iov_cnt",
> +   test_lp1878263_megasas_zero_iov_cnt);
> +}
> +
> +return g_test_run();
> +}
> +
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 874b5be62b..e5de39ffe4 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -54,6 +54,7 @@ qtests_i386 = \
> 'bios-tables-test',
> 'rtc-test',
> 'i440fx-test',
> +   'fuzz-test',
> 'fw_cfg-test',
> 'device-plug-test',
> 'drive_del-test',
> 

You missed to CC: qemu-devel@nongnu.org ... done now and queued to my
qtest-next branch:

 https://gitlab.com/huth/qemu/-/commits/qtest-next/

 Thomas

Re: [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10

[Peter Maydell]

On Sat, 10 Oct 2020 at 08:59, Paolo Bonzini  wrote:
>
> The following changes since commit 4a7c0bd9dcb08798c6f82e55b5a3423f7ee669f1:
>
>   Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-5.2-20201009' 
> into staging (2020-10-09 15:48:04 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 1340ff2adb2624e61c5fcb0eb1889b932b76f669:
>
>   meson: identify more sections of meson.build (2020-10-09 13:19:50 -0400)
>
> 
> * qtest documentation improvements (Eduardo, myself)
> * libqtest event buffering (Maxim)
> * use RCU for list of children of a bus (Maxim)
> * move more files to softmmu/ (myself)
> * meson.build cleanups, qemu-storage-daemon fix (Philippe)
>

This produces a new warning, I think from Sphinx:

Running Sphinx v1.6.7
loading pickled environment... done
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 2 source files that are out of date
updating environment: 1 added, 2 changed, 0 removed
reading sources... [ 33%] index
reading sources... [ 66%] qtest
reading sources... [100%] testing

looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [ 33%] index
writing output... [ 66%] qtest
writing output... [100%] testing

generating indices... genindex
writing additional pages... search
copying static files... done
copying extra files... done
dumping search index in English (code: en) ... done
dumping object inventory... done
build succeeded.
/home/petmay01/linaro/qemu-for-merges/docs/../tests/qtest/libqos/libqtest.h:241:
warning: Function parameter or member 'event' not described in
'qtest_qmp_event_ref'
make: Leaving directory '/home/petmay01/linaro/qemu-for-merges/build/all'
make: Entering directory '/home/petmay01/linaro/qemu-for-merges/build/all'
[build continues and succeeds]

Not sure why it isn't fatal.

thanks
-- PMM

Re: Purpose of QOM properties registered at realize time?

[Mark Cave-Ayland]

On 06/10/2020 23:06, Eduardo Habkost wrote:

> Hi,
> 
> While trying to understand how QOM properties are used in QEMU, I
> stumbled upon multiple cases where alias properties are added at
> realize time.
> 
> Now, I don't understand why those properties exist.  As the
> properties are added at realize time, I assume they aren't
> supposed to be touched by the user at all.  If they are not
> supposed to be touched by the user, what exactly is the purpose
> of those QOM properties?
> 
> For reference, these are the cases I've found:

(cut)

> --
> hw/misc/mac_via.c=1011=static void mac_via_realize(DeviceState *dev, Error 
> **errp)
> hw/misc/mac_via.c:1028:object_property_add_alias(OBJECT(dev), "irq[0]", 
> OBJECT(ms),

This one was me trying to work out how to wire up an IRQ from a child device 
embedded
within the macio device - I'll send a patch to remove it shortly.


ATB,

Mark.

Re: [PATCH 0/3] Acceptance Tests: improve usage on GitLab CI

[Thomas Huth]

On 09/10/2020 22.55, Cleber Rosa wrote:
> This attempts to improve a bit the execution of acceptance tests
> in both GitLab CI, and in restricted Python environments.
> 
> For the GitLab's rendering of test results, a sample can be seen here:
> 
>   https://gitlab.com/cleber.gnu/qemu/-/pipelines/200639030/test_report
> 
> Cleber Rosa (3):
>   Acceptance tests: bump pycdlib version for easier installation
>   Acceptance tests: do not show canceled test logs on GitLab CI
>   Acceptance tests: show test report on GitLab CI
> 
>  .gitlab-ci.yml | 7 ++-
>  tests/requirements.txt | 2 +-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 

Thanks, queued (with the bug URL fixed).

 Thomas

Re: [RFC v1 3/4] qtest: do not build ide-test if TCG is not available

[Claudio Fontana]

On 10/12/20 12:14 PM, Claudio Fontana wrote:
> On 10/10/20 12:50 PM, Claudio Fontana wrote:
>> On 10/9/20 6:01 PM, Paolo Bonzini wrote:
>>> On 09/10/20 17:21, Claudio Fontana wrote:
 it seems that ide-test depends on TCG currently.

 Signed-off-by: Claudio Fontana 
 ---
  tests/qtest/meson.build | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
 index ad33ac311d..3418f65e2a 100644
 --- a/tests/qtest/meson.build
 +++ b/tests/qtest/meson.build
 @@ -46,9 +46,9 @@ qtests_i386 = \
(config_all_devices.has_key('CONFIG_TPM_TIS_ISA') ? ['tpm-tis-test'] : 
 []) +  \
(config_all_devices.has_key('CONFIG_TPM_TIS_ISA') ? 
 ['tpm-tis-swtpm-test'] : []) +\
(config_all_devices.has_key('CONFIG_RTL8139_PCI') ? ['rtl8139-test'] : 
 []) +  \
 +  (config_all.has_key('CONFIG_TCG') ? ['ide-test'] : []) +
   \
qtests_pci +
   \
['fdc-test',
 -   'ide-test',
 'hd-geo-test',
 'boot-order-test',
 'bios-tables-test',

>>>
>>> Interesting, why?...
>>>
>>> Paolo
>>>
>>>
>>
>> I am slowly trying to find out. I found out that the qos-test that buzzes is 
>> ide-test,
>> and I found out which specific ide test it was by manually bisecting 
>> functions inside the qtest_add_func in ide-test.c.
>>
>> The issue seems limited to qtest_add_func("/ide/bmdma/trim", 
>> test_bmdma_trim);
>> No idea yet why that test buzzes forever.
>>
>> Side note, maybe more verbose output on which specific test is attempted 
>> could be helpful? maybe only enabled on make V=2 ?
>>
>> So the buzz.
>> top says:
>>
>> 22621 claudio   20   0   89700   3292   3004 R 53.82 0.010   1:22.43 
>> ide-test  
>> 22844 claudio   20   0 1026700  61168  38632 R 99.67 0.188   2:39.53 
>> qemu-system-i38   
>> 25325 claudio   20   0   89700   3208   2940 R 52.16 0.010   0:56.05 
>> ide-test   
>> 25403 claudio   20   0 1026720  63028  38416 R 99.67 0.194   1:48.63 
>> qemu-system-x86   
>>
>>
>> i386 and x86_64 seem to show the exact same behaviour.
>>
>>
>> gdb says:
>>
>> qemu-system-x86 (25403):
>>
>> (gdb) info threads
>>   Id   Target Id   Frame 
>> * 1Thread 0x7fe35a406140 (LWP 25403) "qemu-system-x86" 
>> 0x7fe35157f7d6 in ppoll () from /lib64/libc.so.6
>>   2Thread 0x7fe33946e700 (LWP 25415) "qemu-system-x86" 
>> 0x7fe351584839 in syscall () from /lib64/libc.so.6
>>   3Thread 0x7fe338c6d700 (LWP 25439) "qemu-system-x86" 
>> 0x7fe35157f6db in poll () from /lib64/libc.so.6
>>   4Thread 0x7fe333fff700 (LWP 25440) "qemu-system-x86" 
>> 0x7fe35185bdcf in do_sigwait () from /lib64/libpthread.so.0
>>
>> (gdb) thread 1
>> [Switching to thread 1 (Thread 0x7fe35a406140 (LWP 25403))]
>> #0  0x7fe35157f7d6 in ppoll () from /lib64/libc.so.6
>> (gdb) bt
>> #0  0x7fe35157f7d6 in ppoll () at /lib64/libc.so.6
>> #1  0x55a1f3138309 in ppoll (__ss=0x0, __timeout=0x7fff64d10b70, 
>> __nfds=, __fds=)
>> at /usr/include/bits/poll2.h:77
>> #2  0x55a1f3138309 in qemu_poll_ns (fds=, nfds=> out>, timeout=timeout@entry=27462700)
>> at ../util/qemu-timer.c:349
>> #3  0x55a1f31512a5 in os_host_main_loop_wait (timeout=27462700) at 
>> ../util/main-loop.c:239
>> #4  0x55a1f31512a5 in main_loop_wait (nonblocking=nonblocking@entry=0) 
>> at ../util/main-loop.c:520
>> #5  0x55a1f2fc4bbd in qemu_main_loop () at ../softmmu/vl.c:1677
>> #6  0x55a1f2d001fe in main (argc=, argv=, 
>> envp=) at ../softmmu/main.c:50
>>
>> (gdb) thread 2
>> [Switching to thread 2 (Thread 0x7fe33946e700 (LWP 25415))]
>> #0  0x7fe351584839 in syscall () from /lib64/libc.so.6
>> (gdb) bt
>> #0  0x7fe351584839 in syscall () at /lib64/libc.so.6
>> #1  0x55a1f312605b in qemu_futex_wait (val=, f=> out>)
>> at /home/claudio/git/qemu-pristine/qemu/include/qemu/futex.h:29
>> #2  0x55a1f312605b in qemu_event_wait (ev=ev@entry=0x55a1f3a44208 
>> ) at ../util/qemu-thread-posix.c:460
>> #3  0x55a1f314f868 in call_rcu_thread (opaque=opaque@entry=0x0) at 
>> ../util/rcu.c:258
>> #4  0x55a1f3125276 in qemu_thread_start (args=) at 
>> ../util/qemu-thread-posix.c:521
>> #5  0x7fe3518514f9 in start_thread () at /lib64/libpthread.so.0
>> #6  0x7fe351589fbf in clone () at /lib64/libc.so.6
>> (gdb) frame 3
>> #3  0x55a1f314f868 in call_rcu_thread (opaque=opaque@entry=0x0) at 
>> ../util/rcu.c:258
>> 258 qemu_event_wait(&rcu_call_ready_event);
>> (gdb) list 258
>> 253 n = qatomic_read(&rcu_call_count);
>> 254 if (n == 0) {
>> 25

Re: [PATCH V13 6/9] target/mips: Add loongson-ext lsdc2 group of instructions

[Huacai Chen]

Hi, Philippe,

On Sun, Oct 11, 2020 at 7:13 PM Philippe Mathieu-Daudé  wrote:
>
> On 10/11/20 5:02 AM, Huacai Chen wrote:
> > Hi, Philippe,
> >
> > On Sat, Oct 10, 2020 at 9:07 PM Philippe Mathieu-Daudé  
> > wrote:
> >>
> >> On 10/7/20 10:39 AM, Huacai Chen wrote:
> >>> From: Jiaxun Yang 
> >>>
> >>> LDC2/SDC2 opcodes have been rewritten as "load & store with offset"
> >>> group of instructions by loongson-ext ASE.
> >>>
> >>> This patch add implementation of these instructions:
> >>> gslbx: load 1 bytes to GPR
> >>> gslhx: load 2 bytes to GPR
> >>> gslwx: load 4 bytes to GPR
> >>> gsldx: load 8 bytes to GPR
> >>> gslwxc1: load 4 bytes to FPR
> >>> gsldxc1: load 8 bytes to FPR
> >>> gssbx: store 1 bytes from GPR
> >>> gsshx: store 2 bytes from GPR
> >>> gsswx: store 4 bytes from GPR
> >>> gssdx: store 8 bytes from GPR
> >>> gsswxc1: store 4 bytes from FPR
> >>> gssdxc1: store 8 bytes from FPR
> >>>
> >>> Details of Loongson-EXT is here:
> >>> https://github.com/FlyGoat/loongson-insn/blob/master/loongson-ext.md
> >>>
> >>> Signed-off-by: Huacai Chen 
> >>> Signed-off-by: Jiaxun Yang 
> >>
> >> If this patch is from Jiaxun, Huacai's S-o-b should come *after*.
> > OK, I will do that.
> >
> >>
> >>> ---
> >>>target/mips/translate.c | 179 
> >>> 
> >>>1 file changed, 179 insertions(+)
> >>>
> >>> diff --git a/target/mips/translate.c b/target/mips/translate.c
> >>> index 916b57f..4d42cfc 100644
> >>> --- a/target/mips/translate.c
> >>> +++ b/target/mips/translate.c
> >>> @@ -484,6 +484,24 @@ enum {
> >>>OPC_GSSDRC1 = 0x7 | OPC_GSSHFS,
> >>>};
> >>>
> >>> +/* Loongson EXT LDC2/SDC2 opcodes */
> >>> +#define MASK_LOONGSON_LSDC2(op)   (MASK_OP_MAJOR(op) | (op & 
> >>> 0x7))
> >>> +
> >>> +enum {
> >>> +OPC_GSLBX  = 0x0 | OPC_LDC2,
> >>> +OPC_GSLHX  = 0x1 | OPC_LDC2,
> >>> +OPC_GSLWX  = 0x2 | OPC_LDC2,
> >>> +OPC_GSLDX  = 0x3 | OPC_LDC2,
> >>> +OPC_GSLWXC1= 0x6 | OPC_LDC2,
> >>> +OPC_GSLDXC1= 0x7 | OPC_LDC2,
> >>> +OPC_GSSBX  = 0x0 | OPC_SDC2,
> >>> +OPC_GSSHX  = 0x1 | OPC_SDC2,
> >>> +OPC_GSSWX  = 0x2 | OPC_SDC2,
> >>> +OPC_GSSDX  = 0x3 | OPC_SDC2,
> >>> +OPC_GSSWXC1= 0x6 | OPC_SDC2,
> >>> +OPC_GSSDXC1= 0x7 | OPC_SDC2,
> >>> +};
> >>> +
> >>>/* BSHFL opcodes */
> >>>#define MASK_BSHFL(op)  (MASK_SPECIAL3(op) | (op & (0x1F 
> >>> << 6)))
> >>>
> >>> @@ -6172,6 +6190,165 @@ static void gen_loongson_lswc2(DisasContext *ctx, 
> >>> int rt,
> >>>tcg_temp_free(t0);
> >>>}
> >>>
> >>> +/* Loongson EXT LDC2/SDC2 */
> >>> +static void gen_loongson_lsdc2(DisasContext *ctx, int rt,
> >>> +int rs, int rd)
> >>
> >> Alignment off (various occurences in this series).
> > OK, thanks.
> >
> >>
> >>> +{
> >>> +int offset = (int8_t)(ctx->opcode >> 3);
> >>
> >> Please use sextract32() which is easier to read:
> >>
> >>  int32_t offset = sextract32(ctx->opcode, 3, 8);
> > OK, thanks.
> >
> >>
> >>> +uint32_t opc = MASK_LOONGSON_LSDC2(ctx->opcode);
> >>> +TCGv t0, t1;
> >>> +TCGv_i32 fp0;
> >>> +
> >>> +/* Pre-conditions */
> >>> +switch (opc) {
> >>> +case OPC_GSLBX:
> >>> +case OPC_GSLHX:
> >>> +case OPC_GSLWX:
> >>> +case OPC_GSLDX:
> >>> +/* prefetch, implement as NOP */
> >>> +if (rt == 0) {
> >>> +return;
> >>> +}
> >>> +break;
> >>> +case OPC_GSSBX:
> >>> +case OPC_GSSHX:
> >>> +case OPC_GSSWX:
> >>> +case OPC_GSSDX:
> >>> +break;
> >>> +case OPC_GSLWXC1:
> >>> +#if defined(TARGET_MIPS64)
> >>> +case OPC_GSLDXC1:
> >>> +#endif
> >>> +check_cp1_enabled(ctx);
> >>> +/* prefetch, implement as NOP */
> >>> +if (rt == 0) {
> >>> +return;
> >>> +}
> >>> +break;
> >>> +case OPC_GSSWXC1:
> >>> +#if defined(TARGET_MIPS64)
> >>> +case OPC_GSSDXC1:
> >>> +#endif
> >>> +check_cp1_enabled(ctx);
> >>> +break;
> >>> +default:
> >>> +MIPS_INVAL("loongson_lsdc2");
> >>> +generate_exception_end(ctx, EXCP_RI);
> >>> +return;
> >>> +break;
> >>> +}
> >>> +
> >>> +t0 = tcg_temp_new();
> >>> +
> >>> +gen_base_offset_addr(ctx, t0, rs, offset);
> >>> +gen_op_addr_add(ctx, t0, cpu_gpr[rd], t0);
> >>> +
> >>> +switch (opc) {
> >>> +case OPC_GSLBX:
> >>> +tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_SB);
> >>> +gen_store_gpr(t0, rt);
> >>> +break;
> >>> +case OPC_GSLHX:
> >>> +tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESW |
> >>> +ctx->default_tcg_memop_mask);
> >>
> >> Do Loongson EXT plan to support unaligned accesses?
> > Not support in hardware, and Linux kernel emulate the unaligned cases.
>
> OK, that was my understanding. So we don't need to use
> default_tcg_memop_mask, we can directly use 

Re: [PATCH] hw/net: move allocation to the heap due to very large stack frame

[Thomas Huth]

On 12/10/2020 07.30, David Gibson wrote:
> On Sat, Oct 10, 2020 at 08:53:00AM -0700, Elena Afanasova wrote:
>> On Sat, 2020-10-10 at 17:07 +1100, David Gibson wrote:
>>> On Fri, Oct 09, 2020 at 07:02:56AM -0700, Elena Afanasova wrote:
> From 09905773a00e417d3a37c12350d9e55466fdce8a Mon Sep 17 00:00:00
> 2001
 From: Elena Afanasova 
 Date: Fri, 9 Oct 2020 06:41:36 -0700
 Subject: [PATCH] hw/net: move allocation to the heap due to very
 large stack
  frame
>>>
>>> Patch looks fine, but some more details of the motivation would be
>>> nice.  I wouldn't have thought that the size of a network packet
>>> counted as a "very large" stack frame by userspace standards.
>>>
>>
>> gcc with wstack-usage warns that stack frame size may be ~65Kbytes
> 
> AFAICT, -Wstack-usage takes a parameter giving what size it will
> complain about.  What was that value, and what was the rationale for
> choosing it?

I think this is one of the tasks from:

 https://wiki.qemu.org/Contribute/BiteSizedTasks#Compiler-driven_cleanups

It has been added by Paolo in 2016:

 
https://wiki.qemu.org/index.php?title=Contribute/BiteSizedTasks&diff=5368&oldid=5367

... so maybe Paolo can comment on the size that has been chosen here...?

 Thomas

Re: [PULL 09/14] qmp: Move dispatcher to a coroutine

[Alex Bennée]

Markus Armbruster  writes:

> From: Kevin Wolf 
>
> This moves the QMP dispatcher to a coroutine and runs all QMP command
> handlers that declare 'coroutine': true in coroutine context so they
> can avoid blocking the main loop while doing I/O or waiting for other
> events.

This subtly changes the replay behaviour leading to a hang in:

  10:55:18 [alex.bennee@hackbox2:~/l/q/b/bisect] (625581c2…)|✚1(+1/-1) + 
./tests/venv/bin/avocado run 
tests/acceptance/replay_kernel.py:ReplayKernel.test_arm_virt
  Fetching asset from 
tests/acceptance/replay_kernel.py:ReplayKernel.test_arm_virt
  JOB ID : ec11fd2544f06e6c0d421f16afa757b49f7ed734
  JOB LOG: 
/home/alex.bennee/avocado/job-results/job-2020-10-12T11.40-ec11fd2/job.log
   (1/1) tests/acceptance/replay_kernel.py:ReplayKernel.test_arm_virt: ERROR: 
Could not perform graceful shutdown (26.27 s)
  RESULTS: PASS 0 | ERROR 1 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
CANCEL 0
  JOB TIME   : 27.77 s

Looking at the log:

  2020-10-12 11:40:31,426 __init__ L0085 DEBUG| [3.887411] 
rtc-pl031 901.pl031: setting system clock to 2020-10-12 10:40:31 UTC 
(1602499231)
  2020-10-12 11:40:31,428 __init__ L0085 DEBUG| [3.887431] sr_init: 
No PMIC hook to init smartreflex
  2020-10-12 11:40:31,447 __init__ L0085 DEBUG| [3.897193] 
uart-pl011 900.pl011: no DMA platform data
  2020-10-12 11:40:31,460 __init__ L0085 DEBUG| [3.897242] md: 
Waiting for all devices to be available before autodetect
  2020-10-12 11:40:31,462 __init__ L0085 DEBUG| [3.897259] md: If 
you don't use raid, use raid=noautodetect
  2020-10-12 11:40:31,475 __init__ L0085 DEBUG| [3.897819] md: 
Autodetecting RAID arrays.
  2020-10-12 11:40:31,476 __init__ L0085 DEBUG| [3.897832] md: 
autorun ...
  2020-10-12 11:40:31,477 __init__ L0085 DEBUG| [3.897842] md: ... 
autorun DONE.
  2020-10-12 11:40:31,483 __init__ L0085 DEBUG| [3.897962] VFS: 
Cannot open root device "(null)" or unknown-block(0,0): error -6
  2020-10-12 11:40:31,483 qmp  L0245 DEBUG| >>> {'execute': 'quit'}
  2020-10-12 11:40:31,495 qmp  L0145 DEBUG| <<< {'timestamp': 
{'seconds': 1602499231, 'microseconds': 493379}, 'event': 'SHUTDOWN', 'data': 
{'guest': True, 'reason': 'guest-reset'}}
  2020-10-12 11:40:31,733 machine  L0325 WARNI| qemu received signal 6; 
command: "./qemu-system-arm -display none -vga none -chardev 
socket,id=mon,path=/var/tmp/tmpzls53khe/qemu-8487-monitor.sock -mon 
chardev=mon,mode=control -machine virt -chardev 
socket,id=console,path=/var/tmp/tmpzls53khe/qemu-8487-console.sock,server,nowait
 -serial chardev:console -icount 
shift=1,rr=record,rrfile=/var/tmp/avocado_n00stdrf/avocado_job_aw60qdul/1-tests_acceptance_replay_kernel.py_ReplayKernel.test_arm_virt/replay.bin
 -kernel 
/home/alex.bennee/avocado/data/cache/by_location/62750ce9e069e69e6a7ff04ff54c382ee660b92a/vmlinuz
 -append printk.time=1 panic=-1 console=ttyAMA0 -net none -no-reboot"
  2020-10-12 11:40:31,734 stacktrace   L0039 ERROR|
  2020-10-12 11:40:31,734 stacktrace   L0042 ERROR| Reproduced traceback 
from: 
/home/alex.bennee/lsrc/qemu.git/builds/bisect/tests/venv/lib/python3.6/site-packages/avocado/core/test.py:767
  2020-10-12 11:40:31,735 stacktrace   L0045 ERROR| Traceback (most recent 
call last):
  2020-10-12 11:40:31,735 stacktrace   L0045 ERROR|   File 
"/home/alex.bennee/lsrc/qemu.git/python/qemu/machine.py", line 435, in 
_do_shutdown
  2020-10-12 11:40:31,735 stacktrace   L0045 ERROR| 
self._soft_shutdown(timeout, has_quit)
  2020-10-12 11:40:31,735 stacktrace   L0045 ERROR|   File 
"/home/alex.bennee/lsrc/qemu.git/python/qemu/machine.py", line 415, in 
_soft_shutdown
  2020-10-12 11:40:31,735 stacktrace   L0045 ERROR| 
self._qmp.cmd('quit')
  2020-10-12 11:40:31,735 stacktrace   L0045 ERROR|   File 
"/home/alex.bennee/lsrc/qemu.git/python/qemu/qmp.py", line 266, in cmd
  2020-10-12 11:40:31,735 stacktrace   L0045 ERROR| return 
self.cmd_obj(qmp_cmd)
  2020-10-12 11:40:31,735 stacktrace   L0045 ERROR|   File 
"/home/alex.bennee/lsrc/qemu.git/python/qemu/qmp.py", line 249, in cmd_obj
  2020-10-12 11:40:31,736 stacktrace   L0045 ERROR| raise 
QMPConnectError("Unexpected empty reply from server")
  2020-10-12 11:40:31,736 stacktrace   L0045 ERROR| 
qemu.qmp.QMPConnectError: Unexpected empty reply from server
  2020-10-12 11:40:31,736 stacktrace   L0045 ERROR|
  2020-10-12 11:40:31,736 stacktrace   L0045 ERROR| The above exception was 
the direct cause of the following exception:
  2020-10-12 11:40:31,736 stacktrace   L0045 ERROR|
  2020-10-12 11:40:31,736 stacktrace   L0045 ERROR| Traceback (most recent 
call last):
  2020-10-12 11:40:31,736 stacktrace   L0045 ERROR|   File 
"/home/alex.bennee/lsrc/qemu.git/builds/bisect/tests/acceptance/replay_kernel.py",
 line 128, in test_arm_virt
  2020-10-12 11:40:31,

Re: [PATCH 3/4] hw/pci-host/prep: Fix PCI swizzling in map_irq()

[Philippe Mathieu-Daudé]

On 10/12/20 11:37 AM, Mark Cave-Ayland wrote:

On 12/10/2020 08:19, Philippe Mathieu-Daudé wrote:


In commit a01d8cadadf we changed the number of IRQs to 4 but
forgot to update the map_irq() function. Do it now.

Fixes: a01d8cadadf ("Fix memory corruption ... in PreP emulation")
Signed-off-by: Philippe Mathieu-Daudé 
---
Cc: Jocelyn Mayer 
Cc: Julian Seward 
Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/pci-host/prep.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 064593d1e52..2224135fedb 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -195,7 +195,7 @@ static const MemoryRegionOps raven_io_ops = {
  
  static int raven_map_irq(PCIDevice *pci_dev, int irq_num)

  {
-return (irq_num + (pci_dev->devfn >> 3)) & 1;
+return (irq_num + (pci_dev->devfn >> 3)) & 3;
  }
  
  static void raven_set_irq(void *opaque, int irq_num, int level)


It feels like this should also have a corresponding change in OpenBIOS for
consistency, even though technically because of the OR on IRQ 15 it doesn't 
really
matter. The relevant part in OpenBIOS can be found here:
https://git.qemu.org/?p=openbios.git;a=blob;f=drivers/pci.c;h=34ae69a907b6312a3a7ab218afe8ba9efded1df7;hb=7f28286f5cb1ca682e3ba0a8706d8884f12bc49e#l2001
and in particular this section:

 /* Use the same "physical" routing as QEMU's raven_map_irq() although
ultimately all 4 PCI interrupts are ORd to IRQ 15 as indicated
by the PReP specification */
 props[(*ncells)++] = arch->irqs[((intno - 1) + (addr >> 11)) & 1];


Done:
https://github.com/openbios/openbios/pull/7




ATB,

Mark.

Re: [PATCH V13 6/9] target/mips: Add loongson-ext lsdc2 group of instructions

[Philippe Mathieu-Daudé]

On 10/12/20 12:33 PM, Huacai Chen wrote:

Hi, Philippe,

On Sun, Oct 11, 2020 at 7:13 PM Philippe Mathieu-Daudé  wrote:


On 10/11/20 5:02 AM, Huacai Chen wrote:

Hi, Philippe,

On Sat, Oct 10, 2020 at 9:07 PM Philippe Mathieu-Daudé  wrote:


On 10/7/20 10:39 AM, Huacai Chen wrote:

From: Jiaxun Yang 

LDC2/SDC2 opcodes have been rewritten as "load & store with offset"
group of instructions by loongson-ext ASE.

This patch add implementation of these instructions:
gslbx: load 1 bytes to GPR
gslhx: load 2 bytes to GPR
gslwx: load 4 bytes to GPR
gsldx: load 8 bytes to GPR
gslwxc1: load 4 bytes to FPR
gsldxc1: load 8 bytes to FPR
gssbx: store 1 bytes from GPR
gsshx: store 2 bytes from GPR
gsswx: store 4 bytes from GPR
gssdx: store 8 bytes from GPR
gsswxc1: store 4 bytes from FPR
gssdxc1: store 8 bytes from FPR

Details of Loongson-EXT is here:
https://github.com/FlyGoat/loongson-insn/blob/master/loongson-ext.md

Signed-off-by: Huacai Chen 
Signed-off-by: Jiaxun Yang 


If this patch is from Jiaxun, Huacai's S-o-b should come *after*.

OK, I will do that.




---
target/mips/translate.c | 179 

1 file changed, 179 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 916b57f..4d42cfc 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -484,6 +484,24 @@ enum {
OPC_GSSDRC1 = 0x7 | OPC_GSSHFS,
};

+/* Loongson EXT LDC2/SDC2 opcodes */
+#define MASK_LOONGSON_LSDC2(op)   (MASK_OP_MAJOR(op) | (op & 0x7))
+
+enum {
+OPC_GSLBX  = 0x0 | OPC_LDC2,
+OPC_GSLHX  = 0x1 | OPC_LDC2,
+OPC_GSLWX  = 0x2 | OPC_LDC2,
+OPC_GSLDX  = 0x3 | OPC_LDC2,
+OPC_GSLWXC1= 0x6 | OPC_LDC2,
+OPC_GSLDXC1= 0x7 | OPC_LDC2,
+OPC_GSSBX  = 0x0 | OPC_SDC2,
+OPC_GSSHX  = 0x1 | OPC_SDC2,
+OPC_GSSWX  = 0x2 | OPC_SDC2,
+OPC_GSSDX  = 0x3 | OPC_SDC2,
+OPC_GSSWXC1= 0x6 | OPC_SDC2,
+OPC_GSSDXC1= 0x7 | OPC_SDC2,
+};
+
/* BSHFL opcodes */
#define MASK_BSHFL(op)  (MASK_SPECIAL3(op) | (op & (0x1F << 6)))

@@ -6172,6 +6190,165 @@ static void gen_loongson_lswc2(DisasContext *ctx, int 
rt,
tcg_temp_free(t0);
}

+/* Loongson EXT LDC2/SDC2 */
+static void gen_loongson_lsdc2(DisasContext *ctx, int rt,
+int rs, int rd)


Alignment off (various occurences in this series).

OK, thanks.




+{
+int offset = (int8_t)(ctx->opcode >> 3);


Please use sextract32() which is easier to read:

  int32_t offset = sextract32(ctx->opcode, 3, 8);

OK, thanks.




+uint32_t opc = MASK_LOONGSON_LSDC2(ctx->opcode);
+TCGv t0, t1;
+TCGv_i32 fp0;
+
+/* Pre-conditions */
+switch (opc) {
+case OPC_GSLBX:
+case OPC_GSLHX:
+case OPC_GSLWX:
+case OPC_GSLDX:
+/* prefetch, implement as NOP */
+if (rt == 0) {
+return;
+}
+break;
+case OPC_GSSBX:
+case OPC_GSSHX:
+case OPC_GSSWX:
+case OPC_GSSDX:
+break;
+case OPC_GSLWXC1:
+#if defined(TARGET_MIPS64)
+case OPC_GSLDXC1:
+#endif
+check_cp1_enabled(ctx);
+/* prefetch, implement as NOP */
+if (rt == 0) {
+return;
+}
+break;
+case OPC_GSSWXC1:
+#if defined(TARGET_MIPS64)
+case OPC_GSSDXC1:
+#endif
+check_cp1_enabled(ctx);
+break;
+default:
+MIPS_INVAL("loongson_lsdc2");
+generate_exception_end(ctx, EXCP_RI);
+return;
+break;
+}
+
+t0 = tcg_temp_new();
+
+gen_base_offset_addr(ctx, t0, rs, offset);
+gen_op_addr_add(ctx, t0, cpu_gpr[rd], t0);
+
+switch (opc) {
+case OPC_GSLBX:
+tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_SB);
+gen_store_gpr(t0, rt);
+break;
+case OPC_GSLHX:
+tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESW |
+ctx->default_tcg_memop_mask);


Do Loongson EXT plan to support unaligned accesses?

Not support in hardware, and Linux kernel emulate the unaligned cases.


OK, that was my understanding. So we don't need to use
default_tcg_memop_mask, we can directly use MO_ALIGN in
place instead.

I read the code again, and found that MIPSR6 uses MO_UNALN while
others use MO_ALIGN. And I also realized that Loongson-3A4000 supports
unaligned access in hardware (the same as R6). So, I think I should
keep default_tcg_memop_mask here. And if possible, I want to set
MO_UNALN for all Loongson-3 processors.


OK.



Huacai


Regards,

Phil.

Re: [PATCH] hw/net: move allocation to the heap due to very large stack frame

[Philippe Mathieu-Daudé]

On 10/10/20 8:07 AM, David Gibson wrote:

On Fri, Oct 09, 2020 at 07:02:56AM -0700, Elena Afanasova wrote:

>From 09905773a00e417d3a37c12350d9e55466fdce8a Mon Sep 17 00:00:00 2001
From: Elena Afanasova 
Date: Fri, 9 Oct 2020 06:41:36 -0700
Subject: [PATCH] hw/net: move allocation to the heap due to very large stack
  frame


Patch looks fine, but some more details of the motivation would be
nice.  I wouldn't have thought that the size of a network packet
counted as a "very large" stack frame by userspace standards.


Maybe academia doing research on "super jumbo frames"?

"Super jumbo frames ... increase the path MTU of high-performance
national research and education networks from 1500 bytes to 9000
bytes or so, a subsequent increase, possibly to 64,000 bytes"

(https://en.wikipedia.org/wiki/Jumbo_frame#Super_jumbo_frames)




Signed-off-by: Elena Afanasova 
---
  hw/net/spapr_llan.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
index 2093f1bad0..581320a0e7 100644
--- a/hw/net/spapr_llan.c
+++ b/hw/net/spapr_llan.c
@@ -688,7 +688,8 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
  SpaprVioDevice *sdev = spapr_vio_find_by_reg(spapr->vio_bus, reg);
  SpaprVioVlan *dev = VIO_SPAPR_VLAN_DEVICE(sdev);
  unsigned total_len;
-uint8_t *lbuf, *p;
+uint8_t *p;
+g_autofree uint8_t *lbuf = NULL;
  int i, nbufs;
  int ret;
  
@@ -729,7 +730,7 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,

  return H_RESOURCE;
  }
  
-lbuf = alloca(total_len);

+lbuf = g_malloc(total_len);
  p = lbuf;
  for (i = 0; i < nbufs; i++) {
  ret = spapr_vio_dma_read(sdev, VLAN_BD_ADDR(bufs[i]),

Re: [PATCH 1/3] block: push error reporting into bdrv_all_*_snapshot functions

[Max Reitz]

On 12.10.20 12:16, Philippe Mathieu-Daudé wrote:
> On 10/12/20 12:07 PM, Max Reitz wrote:
>> On 08.10.20 19:48, Philippe Mathieu-Daudé wrote:
>>> From: Daniel P. Berrangé 
>>>
>>> The bdrv_all_*_snapshot functions return a BlockDriverState pointer
>>> for the invalid backend, which the callers then use to report an
>>> error message. In some cases multiple callers are reporting the
>>> same error message, but with slightly different text. In the future
>>> there will be more error scenarios for some of these methods, which
>>> will benefit from fine grained error message reporting. So it is
>>> helpful to push error reporting down a level.
>>>
>>> Signed-off-by: Daniel P. Berrangé 
>>> ---
>>>   include/block/snapshot.h   | 14 +++
>>>   block/monitor/block-hmp-cmds.c |  7 ++--
>>>   block/snapshot.c   | 77 +-
>>>   migration/savevm.c | 37 +---
>>>   monitor/hmp-cmds.c |  7 +---
>>>   replay/replay-debugging.c  |  4 +-
>>>   tests/qemu-iotests/267.out | 10 ++---
>>>   7 files changed, 67 insertions(+), 89 deletions(-)
>>
>> Looks good overall to me, but for some reason this patch pulls in the
>> @ok and @ret variables from the top scope of all concerned functions
>> into the inner scopes of the BDS loops, and drops their initialization.
>>   That’s wrong, because we only call the respective snapshotting
>> functions on some BDSs, so the return value stays uninitialized for all
>> other BDSs:
> 
> Indeed, thanks for catching that.
> 
> [...]
>>>   int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
>>>    BlockDriverState *vm_state_bs,
>>>    uint64_t vm_state_size,
>>> - BlockDriverState **first_bad_bs)
>>> + Error **errp)
>>>   {
>>> -    int err = 0;
>>>   BlockDriverState *bs;
>>>   BdrvNextIterator it;
>>>     for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
>>>   AioContext *ctx = bdrv_get_aio_context(bs);
>>> +    int ret;
>>
>> And one final time.
>>
>> Max
>>
>>>   aio_context_acquire(ctx);
>>>   if (bs == vm_state_bs) {
>>>   sn->vm_state_size = vm_state_size;
>>> -    err = bdrv_snapshot_create(bs, sn);
>>> +    ret = bdrv_snapshot_create(bs, sn);
>>>   } else if (bdrv_all_snapshots_includes_bs(bs)) {
>>>   sn->vm_state_size = 0;
>>> -    err = bdrv_snapshot_create(bs, sn);
>>> +    ret = bdrv_snapshot_create(bs, sn);
> 
> This one is not needed.

Why not?  Is bdrv_all_snapshots_includes_bs(bs) guaranteed to be true?
(I don’t see any a plain “else” branch, or where ret would be set
outside of these two “if” blocks.)

Max

>>>   }
>>>   aio_context_release(ctx);
>>> -    if (err < 0) {
>>> +    if (ret < 0) {
>>> +    error_setg(errp, "Could not create snapshot '%s' on '%s'",
>>> +   sn->name, bdrv_get_device_or_node_name(bs));
>>>   bdrv_next_cleanup(&it);
>>> -    goto fail;
>>> +    return -1;
>>>   }
>>>   }
>>
> 

Re: [PATCH] vhost-user: add separate memslot counter for vhost-user

[chenjiajun]

On 2020/10/2 10:05, Raphael Norwitz wrote:
> On Mon, Sep 28, 2020 at 9:17 AM Jiajun Chen  wrote:
>>
>> Used_memslots is equal to dev->mem->nregions now, it is true for
>> vhost kernel, but not for vhost user, which uses the memory regions
>> that have file descriptor. In fact, not all of the memory regions
>> have file descriptor.
>> It is usefully in some scenarios, e.g. used_memslots is 8, and only
>> 5 memory slots can be used by vhost user, it is failed to hot plug
>> a new memory RAM because vhost_has_free_slot just returned false,
>> but we can hot plug it safely in fact.
>>
>> --
>> ChangeList:
>> v3:
>> -make used_memslots a member of struct vhost_dev instead of a global static 
>> value
>>
>> v2:
>> -eliminating useless used_memslots_exceeded variable and 
>> used_memslots_is_exceeded() API
>>
>> v1:
>> -vhost-user: add separate memslot counter for vhost-user
>>
>> Signed-off-by: Jiajun Chen 
>> Signed-off-by: Jianjay Zhou 
> 
> I'm happy with this from a vhost/vhost-user perspective. vhost-backend
> change looks good too. I'm a little confused by what's going on with
> net/vhost-user.c.
> 
>> ---
>>  hw/virtio/vhost-backend.c | 12 ++
>>  hw/virtio/vhost-user.c| 25 +
>>  hw/virtio/vhost.c | 37 +++
>>  include/hw/virtio/vhost-backend.h |  5 +
>>  include/hw/virtio/vhost.h |  1 +
>>  net/vhost-user.c  |  7 ++
>>  6 files changed, 78 insertions(+), 9 deletions(-)
>>
> 
>> diff --git a/net/vhost-user.c b/net/vhost-user.c
>> index 17532daaf3..7e93955537 100644
>> --- a/net/vhost-user.c
>> +++ b/net/vhost-user.c
>> @@ -20,6 +20,7 @@
>>  #include "qemu/error-report.h"
>>  #include "qemu/option.h"
>>  #include "trace.h"
>> +#include "include/hw/virtio/vhost.h"
>>
>>  typedef struct NetVhostUserState {
>>  NetClientState nc;
>> @@ -347,6 +348,12 @@ static int net_vhost_user_init(NetClientState *peer, 
>> const char *device,
>>  qemu_chr_fe_set_handlers(&s->chr, NULL, NULL,
>>   net_vhost_user_event, NULL, nc0->name, 
>> NULL,
>>   true);
> 
> Can you elaborate on this check here? What does it have to do with
> fixing memslots accounting? Maybe it should be in a separate change?
> 
When the number of virtual machine memslots exceeds the upper limit of the 
back-end support,
QEMU main thread may enters an endless loop and cannot process other requests.
And number of memslots will not automatically decrease, so add a check here to 
exit from loop
in this scenario. For the newly started virtual machine, boot fails; for the 
hot plug network card,
hot plug fails.
>> +
>> +if (!vhost_has_free_slot()) {
>> +error_report("used memslots exceeded the backend limit, quit "
>> + "loop");
>> +goto err;
>> +}
>>  } while (!s->started);
>>
>>  assert(s->vhost_net);
>> --
>> 2.27.0.dirty
>>
> .
> 

Re: [PATCH v7 01/13] qtest: rename qtest_qmp_receive to qtest_qmp_receive_dict

[Thomas Huth]

On 06/10/2020 14.38, Maxim Levitsky wrote:
> In the next patch a new version of qtest_qmp_receive will be
> reintroduced that will buffer received qmp events for later
> consumption in qtest_qmp_eventwait_ref
> 
> No functional change intended.
> 
> Suggested-by: Paolo Bonzini 
> Signed-off-by: Maxim Levitsky 
> ---
>  tests/qtest/ahci-test.c|  4 ++--
>  tests/qtest/device-plug-test.c |  2 +-
>  tests/qtest/drive_del-test.c   |  2 +-
>  tests/qtest/libqos/libqtest.h  |  4 ++--
>  tests/qtest/libqtest.c | 16 
>  tests/qtest/pvpanic-test.c |  2 +-
>  tests/qtest/qmp-test.c | 18 +-
>  7 files changed, 24 insertions(+), 24 deletions(-)


Acked-by: Thomas Huth 

Re: [PATCH v7 02/13] qtest: Reintroduce qtest_qmp_receive

[Thomas Huth]

On 06/10/2020 14.38, Maxim Levitsky wrote:
> The new qtest_qmp_receive buffers all the received qmp events, allowing
> qtest_qmp_eventwait_ref to return them.
> 
> This is intended to solve the race in regard to ordering of qmp events
> vs qmp responses, as soon as the callers start using the new interface.
> 
> In addition to that, define qtest_qmp_event_ref a function which only scans
> the buffer that qtest_qmp_receive stores the events to.
> 
> This is intended for callers that are only interested in events that were
> received during the last call to the qtest_qmp_receive.
> 
> Suggested-by: Paolo Bonzini 
> Signed-off-by: Maxim Levitsky 
> ---
>  tests/qtest/libqos/libqtest.h | 23 
>  tests/qtest/libqtest.c| 49 ++-
>  2 files changed, 71 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
> index a41135fc92..19429a536d 100644
> --- a/tests/qtest/libqos/libqtest.h
> +++ b/tests/qtest/libqos/libqtest.h
> @@ -198,6 +198,16 @@ void qtest_qmp_vsend(QTestState *s, const char *fmt, 
> va_list ap)
>   */
>  QDict *qtest_qmp_receive_dict(QTestState *s);
>  
> +/**
> + * qtest_qmp_receive:
> + * @s: #QTestState instance to operate on.
> + *
> + * Reads a QMP message from QEMU and returns the response.
> + * Buffers all the events received meanwhile, until a
> + * call to qtest_qmp_eventwait
> + */
> +QDict *qtest_qmp_receive(QTestState *s);

Re-introducing qtest_qmp_receive() with different behavior than before will
likely make backports of other later patches a pain, and might also break
other patches that use this function but are not merged yet. Could you
please use a different name for this function instead? Maye
qtest_qmp_receive_buffered() or something like that?

 Thomas

Re: [PATCH v4 0/4] migration/postcopy: Sync faulted addresses after network recovered

[Dr. David Alan Gilbert]

* Dr. David Alan Gilbert (dgilb...@redhat.com) wrote:
> Queued

Hi Peter,
  I've had to unqueue this again unfortunately.
There's something going on with big endian hosts; on a PPC BE host,
it reliably hangs in the recovery test with this set.
(Although I can't see anything relevant to eye).

Dave
P.S. I can point you at an installed host

> * Peter Xu (pet...@redhat.com) wrote:
> > v4:
> > - use "void */ulong" instead of "uint64_t" where proper in patch 3/4 [Dave]
> > 
> > v3:
> > - fix build on 32bit hosts & rebase
> > - remove r-bs for the last 2 patches for Dave due to the changes
> > 
> > v2:
> > - add r-bs for Dave
> > - add patch "migration: Properly destroy variables on incoming side" as 
> > patch 1
> > - destroy page_request_mutex in migration_incoming_state_destroy() too 
> > [Dave]
> > - use WITH_QEMU_LOCK_GUARD in two places where we can [Dave]
> > 
> > We've seen conditional guest hangs on destination VM after postcopy 
> > recovered.
> > However the hang will resolve itself after a few minutes.
> > 
> > The problem is: after a postcopy recovery, the prioritized postcopy queue on
> > the source VM is actually missing.  So all the faulted threads before the
> > postcopy recovery happened will keep halted until (accidentally) the page 
> > got
> > copied by the background precopy migration stream.
> > 
> > The solution is to also refresh this information after postcopy recovery.  
> > To
> > achieve this, we need to maintain a list of faulted addresses on the
> > destination node, so that we can resend the list when necessary.  This work 
> > is
> > done via patch 2-5.
> > 
> > With that, the last thing we need to do is to send this extra information to
> > source VM after recovered.  Very luckily, this synchronization can be
> > "emulated" by sending a bunch of page requests (although these pages have 
> > been
> > sent previously!) to source VM just like when we've got a page fault.  Even 
> > in
> > the 1st version of the postcopy code we'll handle duplicated pages well.  So
> > this fix does not even need a new capability bit and it'll work smoothly on 
> > old
> > QEMUs when we migrate from them to the new QEMUs.
> > 
> > Please review, thanks.
> > 
> > Peter Xu (4):
> >   migration: Pass incoming state into qemu_ufd_copy_ioctl()
> >   migration: Introduce migrate_send_rp_message_req_pages()
> >   migration: Maintain postcopy faulted addresses
> >   migration: Sync requested pages after postcopy recovery
> > 
> >  migration/migration.c| 49 --
> >  migration/migration.h| 21 ++-
> >  migration/postcopy-ram.c | 25 +-
> >  migration/savevm.c   | 57 
> >  migration/trace-events   |  3 +++
> >  5 files changed, 146 insertions(+), 9 deletions(-)
> > 
> > -- 
> > 2.26.2
> > 
> > 
> > 
> -- 
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PULL 09/14] qmp: Move dispatcher to a coroutine

[Kevin Wolf]

Am 12.10.2020 um 12:47 hat Alex Bennée geschrieben:
> 
> Markus Armbruster  writes:
> 
> > From: Kevin Wolf 
> >
> > This moves the QMP dispatcher to a coroutine and runs all QMP command
> > handlers that declare 'coroutine': true in coroutine context so they
> > can avoid blocking the main loop while doing I/O or waiting for other
> > events.
> 
> This subtly changes the replay behaviour leading to a hang in:
> 
>   10:55:18 [alex.bennee@hackbox2:~/l/q/b/bisect] (625581c2…)|✚1(+1/-1) + 
> ./tests/venv/bin/avocado run 
> tests/acceptance/replay_kernel.py:ReplayKernel.test_arm_virt
>   Fetching asset from 
> tests/acceptance/replay_kernel.py:ReplayKernel.test_arm_virt
>   JOB ID : ec11fd2544f06e6c0d421f16afa757b49f7ed734
>   JOB LOG: 
> /home/alex.bennee/avocado/job-results/job-2020-10-12T11.40-ec11fd2/job.log
>(1/1) tests/acceptance/replay_kernel.py:ReplayKernel.test_arm_virt: ERROR: 
> Could not perform graceful shutdown (26.27 s)
>   RESULTS: PASS 0 | ERROR 1 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
> CANCEL 0
>   JOB TIME   : 27.77 s
> 
> Looking at the log:
> 
>   2020-10-12 11:40:31,426 __init__ L0085 DEBUG| [3.887411] 
> rtc-pl031 901.pl031: setting system clock to 2020-10-12 10:40:31 UTC 
> (1602499231)
>   2020-10-12 11:40:31,428 __init__ L0085 DEBUG| [3.887431] 
> sr_init: No PMIC hook to init smartreflex
>   2020-10-12 11:40:31,447 __init__ L0085 DEBUG| [3.897193] 
> uart-pl011 900.pl011: no DMA platform data
>   2020-10-12 11:40:31,460 __init__ L0085 DEBUG| [3.897242] md: 
> Waiting for all devices to be available before autodetect
>   2020-10-12 11:40:31,462 __init__ L0085 DEBUG| [3.897259] md: If 
> you don't use raid, use raid=noautodetect
>   2020-10-12 11:40:31,475 __init__ L0085 DEBUG| [3.897819] md: 
> Autodetecting RAID arrays.
>   2020-10-12 11:40:31,476 __init__ L0085 DEBUG| [3.897832] md: 
> autorun ...
>   2020-10-12 11:40:31,477 __init__ L0085 DEBUG| [3.897842] md: 
> ... autorun DONE.
>   2020-10-12 11:40:31,483 __init__ L0085 DEBUG| [3.897962] VFS: 
> Cannot open root device "(null)" or unknown-block(0,0): error -6
>   2020-10-12 11:40:31,483 qmp  L0245 DEBUG| >>> {'execute': 
> 'quit'}
>   2020-10-12 11:40:31,495 qmp  L0145 DEBUG| <<< {'timestamp': 
> {'seconds': 1602499231, 'microseconds': 493379}, 'event': 'SHUTDOWN', 'data': 
> {'guest': True, 'reason': 'guest-reset'}}
>   2020-10-12 11:40:31,733 machine  L0325 WARNI| qemu received signal 
> 6; command: "./qemu-system-arm -display none -vga none -chardev 
> socket,id=mon,path=/var/tmp/tmpzls53khe/qemu-8487-monitor.sock -mon 
> chardev=mon,mode=control -machine virt -chardev 
> socket,id=console,path=/var/tmp/tmpzls53khe/qemu-8487-console.sock,server,nowait
>  -serial chardev:console -icount 
> shift=1,rr=record,rrfile=/var/tmp/avocado_n00stdrf/avocado_job_aw60qdul/1-tests_acceptance_replay_kernel.py_ReplayKernel.test_arm_virt/replay.bin
>  -kernel 
> /home/alex.bennee/avocado/data/cache/by_location/62750ce9e069e69e6a7ff04ff54c382ee660b92a/vmlinuz
>  -append printk.time=1 panic=-1 console=ttyAMA0 -net none -no-reboot"

This looks like a crash (SIGABRT) rather than a hang. Do you have a
stack trace for the crashed process?

Kevin

>   2020-10-12 11:40:31,734 stacktrace   L0039 ERROR|
>   2020-10-12 11:40:31,734 stacktrace   L0042 ERROR| Reproduced traceback 
> from: 
> /home/alex.bennee/lsrc/qemu.git/builds/bisect/tests/venv/lib/python3.6/site-packages/avocado/core/test.py:767
>   2020-10-12 11:40:31,735 stacktrace   L0045 ERROR| Traceback (most 
> recent call last):
>   2020-10-12 11:40:31,735 stacktrace   L0045 ERROR|   File 
> "/home/alex.bennee/lsrc/qemu.git/python/qemu/machine.py", line 435, in 
> _do_shutdown
>   2020-10-12 11:40:31,735 stacktrace   L0045 ERROR| 
> self._soft_shutdown(timeout, has_quit)
>   2020-10-12 11:40:31,735 stacktrace   L0045 ERROR|   File 
> "/home/alex.bennee/lsrc/qemu.git/python/qemu/machine.py", line 415, in 
> _soft_shutdown
>   2020-10-12 11:40:31,735 stacktrace   L0045 ERROR| 
> self._qmp.cmd('quit')
>   2020-10-12 11:40:31,735 stacktrace   L0045 ERROR|   File 
> "/home/alex.bennee/lsrc/qemu.git/python/qemu/qmp.py", line 266, in cmd
>   2020-10-12 11:40:31,735 stacktrace   L0045 ERROR| return 
> self.cmd_obj(qmp_cmd)
>   2020-10-12 11:40:31,735 stacktrace   L0045 ERROR|   File 
> "/home/alex.bennee/lsrc/qemu.git/python/qemu/qmp.py", line 249, in cmd_obj
>   2020-10-12 11:40:31,736 stacktrace   L0045 ERROR| raise 
> QMPConnectError("Unexpected empty reply from server")
>   2020-10-12 11:40:31,736 stacktrace   L0045 ERROR| 
> qemu.qmp.QMPConnectError: Unexpected empty reply from server
>   2020-10-12 11:40:31,736 stacktrace   L0045 ERROR|
>   2020-10-12 11:40:31,736 stacktrace   L0045 ERROR| The above exception 
> was the direct cause of the following exceptio

[PATCH] spapr: Clarify why DR connectors aren't user creatable

[Greg Kurz]

DR connector is a device that emulates a firmware abstraction used by PAPR
compliant guests to manage hotplug/dynamic-reconfiguration of PHBs, PCI
devices, memory, and CPUs.

It is internally created by the spapr platform and requires to be owned by
either the machine (PHBs, CPUs, memory) or by a PHB (PCI devices).

Signed-off-by: Greg Kurz 
---
 hw/ppc/spapr_drc.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 697b28c34305..77718cde1ff2 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -586,7 +586,8 @@ static void spapr_dr_connector_class_init(ObjectClass *k, 
void *data)
 dk->realize = realize;
 dk->unrealize = unrealize;
 /*
- * Reason: it crashes FIXME find and document the real reason
+ * Reason: DR connector needs to be wired to either the machine or to a
+ * PHB in spapr_dr_connector_new().
  */
 dk->user_creatable = false;
 }

Re: [PULL 00/34] QAPI patches patches for 2020-10-10

[Peter Maydell]

On Sat, 10 Oct 2020 at 10:55, Markus Armbruster  wrote:
>
> The following changes since commit 4a7c0bd9dcb08798c6f82e55b5a3423f7ee669f1:
>
>   Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-5.2-20201009' 
> into staging (2020-10-09 15:48:04 +0100)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2020-10-10
>
> for you to fetch changes up to b4c0aa59aff520e2a55edd5fef393058ca6520de:
>
>   qapi/visit.py: add type hint annotations (2020-10-10 11:37:49 +0200)
>
> 
> QAPI patches patches for 2020-10-10
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM

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

[BALATON Zoltan via]

On Mon, 12 Oct 2020, Alexey Kardashevskiy wrote:

On 29/09/2020 20:35, Alexey Kardashevskiy wrote:


On 16/07/2020 23:22, David Gibson wrote:

On Thu, Jul 16, 2020 at 07:04:56PM +1000, Alexey Kardashevskiy wrote:

Ping? I kinda realize it is not going to replace SLOF any time soon but
still...


Yeah, I know.   I just haven't had time to consider it.  Priority
starvation.



Still? :)


Ping?


+1, I'd like to see this merged and experiment with it to emulate firmware 
for pegasos2 but I'd rather use the final version than something off-tree 
which may end up different when gets upstream. Is there a way I could help 
with this?


Regards,
BALATON Zoltan

Re: [PATCH v4 1/7] keyval: Fix and clarify grammar

[Eric Blake]

On 10/11/20 2:34 AM, Markus Armbruster wrote:

The grammar has a few issues:

* key-fragment = / [^=,.]* /

   Prose restricts key fragments: they "must be valid QAPI names or
   consist only of decimal digits".  Technically, '' consists only of
   decimal digits.  The code rejects that.  Fix the grammar.

* val  = { / [^,]* / | ',,' }

   Use + instead of *.  Accepts the same language.

* val-no-key   = / [^=,]* /

   The code rejects an empty value.  Fix the grammar.

* Section "Additional syntax for use with an implied key" is
   confusing.  Rewrite it.

Signed-off-by: Markus Armbruster 
---
  util/keyval.c | 16 ++--
  1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/util/keyval.c b/util/keyval.c
index 13def4af54..82d8497c71 100644
--- a/util/keyval.c
+++ b/util/keyval.c
@@ -16,8 +16,8 @@
   *   key-vals = [ key-val { ',' key-val } [ ',' ] ]
   *   key-val  = key '=' val
   *   key  = key-fragment { '.' key-fragment }
- *   key-fragment = / [^=,.]* /
- *   val  = { / [^,]* / | ',,' }
+ *   key-fragment = / [^=,.]+ /


This requires a non-empty string.  Good (we don't allow an empty key).


+ *   val  = { / [^,]+ / | ',,' }


I agree that this has no real change.  Previously, you allowed zero or 
more repetitions of a regex that could produce zero characters; now, 
each outer repetition must make progress.



   *
   * Semantics defined by reduction to JSON:
   *
@@ -71,12 +71,16 @@
   * Awkward.  Note that we carefully restrict alternate types to avoid
   * similar ambiguity.
   *
- * Additional syntax for use with an implied key:
+ * Alternative syntax for use with an implied key:
   *
- *   key-vals-ik  = val-no-key [ ',' key-vals ]
- *   val-no-key   = / [^=,]* /
+ *   key-vals = [ key-val-1st { ',' key-val } [ ',' ] ]
+ *   key-val-1st  = val-no-key | key-val
+ *   val-no-key   = / [^=,]+ /
   *
- * where no-key is syntactic sugar for implied-key=val-no-key.
+ * where val-no-key is syntactic sugar for implied-key=val-no-key.
+ *
+ * Note that you can't use the sugared form when the value contains
+ * '=' or ','.


Nor can you use the sugared form when the value is intended to be empty 
(although this may be academic, as your other patches enumerate the list 
of clients, and none of them seem to allow an empty value even when 
desugared).


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[PULL 1/6] virtiofsd: Silence gcc warning

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

Gcc worries fd might be used unset, in reality it's always set if
fi is set, and only used if fi is set so it's safe.  Initialise it to -1
just to keep gcc happy for now.

Signed-off-by: Dr. David Alan Gilbert 
Message-Id: <20200827153657.111098-2-dgilb...@redhat.com>
Reviewed-by: Ján Tomko 
Signed-off-by: Dr. David Alan Gilbert 
---
 tools/virtiofsd/passthrough_ll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 0b229ebd57..36ad46e0c0 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -620,7 +620,7 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, 
struct stat *attr,
 struct lo_inode *inode;
 int ifd;
 int res;
-int fd;
+int fd = -1;
 
 inode = lo_inode(req, ino);
 if (!inode) {
-- 
2.28.0

Re: [PATCH v4 2/7] test-keyval: Demonstrate misparse of ',' with implied key

[Eric Blake]

On 10/11/20 2:35 AM, Markus Armbruster wrote:

Add a test for "val,,ue" with implied key.  Documentation says this
should parse as implied key with value "val", then fail.  The code
parses it as implied key with value "val,ue", then succeeds.  The next
commit will fix it.

Signed-off-by: Markus Armbruster 
---
  tests/test-keyval.c | 7 +++
  1 file changed, 7 insertions(+)


Reviewed-by: Eric Blake 



diff --git a/tests/test-keyval.c b/tests/test-keyval.c
index e331a84149..f02bdf7029 100644
--- a/tests/test-keyval.c
+++ b/tests/test-keyval.c
@@ -182,6 +182,13 @@ static void test_keyval_parse(void)
  error_free_or_abort(&err);
  g_assert(!qdict);
  
+/* Implied key's value can't have comma (qemu_opts_parse(): it can) */

+/* BUG: it can */
+qdict = keyval_parse("val,,ue", "implied", &error_abort);
+g_assert_cmpuint(qdict_size(qdict), ==, 1);
+g_assert_cmpstr(qdict_get_try_str(qdict, "implied"), ==, "val,ue");
+qobject_unref(qdict);
+
  /* Empty key is not an implied key */
  qdict = keyval_parse("=val", "implied", &err);
  error_free_or_abort(&err);



--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[PULL 0/6] migration queue

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

The following changes since commit 2387df497b4b4bcf754eb7398edca82889e2ef54:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2020-10-10' into 
staging (2020-10-12 11:29:42 +0100)

are available in the Git repository at:

  git://github.com/dagrh/qemu.git tags/pull-migration-20201012a

for you to fetch changes up to b1a859cfb04db1d2b80dd9979ce6081cb9c00d75:

  migration/dirtyrate: present dirty rate only when querying the rate has 
completed (2020-10-12 12:39:38 +0100)


v3 Migration+ virtiofsd pull 2020-10-12

V3
  Remove the postcopy recovery changes

Migration:
  Dirtyrate measurement API cleanup

Virtiofsd:
  Missing qemu_init_exec_dir call
  Support for setting the group on socket creation
  Stop a gcc warning
  Avoid tempdir in sandboxing


Alex Bennée (1):
  tools/virtiofsd: add support for --socket-group

Chuan Zheng (2):
  migration/dirtyrate: record start_time and calc_time while at the 
measuring state
  migration/dirtyrate: present dirty rate only when querying the rate has 
completed

Dr. David Alan Gilbert (2):
  virtiofsd: Silence gcc warning
  virtiofsd: Call qemu_init_exec_dir

Stefan Hajnoczi (1):
  virtiofsd: avoid /proc/self/fd tempdir

 docs/tools/virtiofsd.rst |  4 
 migration/dirtyrate.c| 16 ++--
 qapi/migration.json  |  8 +++-
 tools/virtiofsd/fuse_i.h |  1 +
 tools/virtiofsd/fuse_lowlevel.c  |  6 ++
 tools/virtiofsd/fuse_virtio.c| 21 +++--
 tools/virtiofsd/passthrough_ll.c | 38 ++
 7 files changed, 57 insertions(+), 37 deletions(-)

[PULL 6/6] migration/dirtyrate: present dirty rate only when querying the rate has completed

[Dr. David Alan Gilbert (git)]

From: Chuan Zheng 

Make dirty_rate field optional, present dirty rate only when querying
the rate has completed.
The qmp results is shown as follow:
@unstarted:
{"return":{"status":"unstarted","start-time":0,"calc-time":0},"id":"libvirt-12"}
@measuring:
{"return":{"status":"measuring","start-time":102931,"calc-time":1},"id":"libvirt-85"}
@measured:
{"return":{"status":"measured","dirty-rate":4,"start-time":150146,"calc-time":1},"id":"libvirt-15"}

Signed-off-by: Chuan Zheng 
Reviewed-by: David Edmondson 
Message-Id: <1601350938-128320-3-git-send-email-zhengch...@huawei.com>
Signed-off-by: Dr. David Alan Gilbert 
---
 migration/dirtyrate.c | 3 +--
 qapi/migration.json   | 8 +++-
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 40e41e793e..ab9e1301f6 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -69,9 +69,8 @@ static struct DirtyRateInfo *query_dirty_rate_info(void)
 struct DirtyRateInfo *info = g_malloc0(sizeof(DirtyRateInfo));
 
 if (qatomic_read(&CalculatingState) == DIRTY_RATE_STATUS_MEASURED) {
+info->has_dirty_rate = true;
 info->dirty_rate = dirty_rate;
-} else {
-info->dirty_rate = -1;
 }
 
 info->status = CalculatingState;
diff --git a/qapi/migration.json b/qapi/migration.json
index 7f5e6fd681..974021a5c8 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1743,10 +1743,8 @@
 #
 # Information about current dirty page rate of vm.
 #
-# @dirty-rate: @dirtyrate describing the dirty page rate of vm
-#  in units of MB/s.
-#  If this field returns '-1', it means querying has not
-#  yet started or completed.
+# @dirty-rate: an estimate of the dirty page rate of the VM in units of
+#  MB/s, present only when estimating the rate has completed.
 #
 # @status: status containing dirtyrate query status includes
 #  'unstarted' or 'measuring' or 'measured'
@@ -1759,7 +1757,7 @@
 #
 ##
 { 'struct': 'DirtyRateInfo',
-  'data': {'dirty-rate': 'int64',
+  'data': {'*dirty-rate': 'int64',
'status': 'DirtyRateStatus',
'start-time': 'int64',
'calc-time': 'int64'} }
-- 
2.28.0

[PULL 3/6] virtiofsd: Call qemu_init_exec_dir

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

Since fcb4f59c879 qemu_get_local_state_pathname relies on the
init_exec_dir, and virtiofsd asserts because we never set it.
Set it.

Reported-by: Alex Bennée 
Signed-off-by: Dr. David Alan Gilbert 
Message-Id: <20201002124015.44820-1-dgilb...@redhat.com>
Tested-by: Alex Bennée 
Reviewed-by: Alex Bennée 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Dr. David Alan Gilbert 
---
 tools/virtiofsd/passthrough_ll.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 36ad46e0c0..477e6ee0b5 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2839,6 +2839,8 @@ int main(int argc, char *argv[])
 /* Don't mask creation mode, kernel already did that */
 umask(0);
 
+qemu_init_exec_dir(argv[0]);
+
 pthread_mutex_init(&lo.mutex, NULL);
 lo.inodes = g_hash_table_new(lo_key_hash, lo_key_equal);
 lo.root.fd = -1;
-- 
2.28.0

[PULL 5/6] migration/dirtyrate: record start_time and calc_time while at the measuring state

[Dr. David Alan Gilbert (git)]

From: Chuan Zheng 

Querying could include both the start-time and the calc-time while at the 
measuring
state, allowing a caller to determine when they should expect to come back 
looking
for a result.

Signed-off-by: Chuan Zheng 
Message-Id: <1601350938-128320-2-git-send-email-zhengch...@huawei.com>
Reviewed-by: David Edmondson 
Signed-off-by: Dr. David Alan Gilbert 
---
 migration/dirtyrate.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 68577ef250..40e41e793e 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -83,14 +83,14 @@ static struct DirtyRateInfo *query_dirty_rate_info(void)
 return info;
 }
 
-static void reset_dirtyrate_stat(void)
+static void init_dirtyrate_stat(int64_t start_time, int64_t calc_time)
 {
 DirtyStat.total_dirty_samples = 0;
 DirtyStat.total_sample_count = 0;
 DirtyStat.total_block_mem_MB = 0;
 DirtyStat.dirty_rate = -1;
-DirtyStat.start_time = 0;
-DirtyStat.calc_time = 0;
+DirtyStat.start_time = start_time;
+DirtyStat.calc_time = calc_time;
 }
 
 static void update_dirtyrate_stat(struct RamblockDirtyInfo *info)
@@ -335,7 +335,6 @@ static void calculate_dirtyrate(struct DirtyRateConfig 
config)
 int64_t initial_time;
 
 rcu_register_thread();
-reset_dirtyrate_stat();
 rcu_read_lock();
 initial_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
 if (!record_ramblock_hash_info(&block_dinfo, config, &block_count)) {
@@ -365,6 +364,8 @@ void *get_dirtyrate_thread(void *arg)
 {
 struct DirtyRateConfig config = *(struct DirtyRateConfig *)arg;
 int ret;
+int64_t start_time;
+int64_t calc_time;
 
 ret = dirtyrate_set_state(&CalculatingState, DIRTY_RATE_STATUS_UNSTARTED,
   DIRTY_RATE_STATUS_MEASURING);
@@ -373,6 +374,10 @@ void *get_dirtyrate_thread(void *arg)
 return NULL;
 }
 
+start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) / 1000;
+calc_time = config.sample_period_seconds;
+init_dirtyrate_stat(start_time, calc_time);
+
 calculate_dirtyrate(config);
 
 ret = dirtyrate_set_state(&CalculatingState, DIRTY_RATE_STATUS_MEASURING,
-- 
2.28.0

Re: Which qemu change corresponds to RedHat bug 1655408

[Max Reitz]

On 09.10.20 14:55, Jakob Bohm wrote:
> On 2020-10-09 10:48, Max Reitz wrote:
>> On 08.10.20 18:49, Jakob Bohm wrote:
>>> (Top posting because previous reply did so):
>>>
>>> If the bug was closed as "can't reproduce", why was a very similar bug
>>> listed as fixed in RHSA-2019:2553-01 ?
>>
>> Hi,
>>
>> Which very similar bug do you mean?  I can only guess that perhaps you
>> mean 1603104 or 1551486.
>>
>> Bug 1603104 was about qemu not ignoring errors when releasing file locks
>> fails (we should ignore errors then, because they're not fatal, and we
>> often cannot return errors, so they ended up as aborts).  (To give more
>> context, this error generally appeared only when the storage the image
>> is on somehow disappeared while qemu is running.  E.g. when the
>> connection to an NFS server was lost.)
>>
>> Bug 1551486 entailed a bit of a rewrite of the whole locking code, which
>> may have resulted in the bug 1655408 no longer appearing for our QE
>> team.  But it was a different bug, as it wasn’t about any error, but
>> just about the fact that qemu used more FDs than necessary.
>>
>> (Although I see 1655408 was reported for RHEL 8, whereas 1603104 and
>> 1551486 (as part of RHSA-2019:2553) were reported for RHEL 7.  The
>> corresponding RHEL 8 bug for those two is 1694148.)
>>
>> Either way, both of those bugs are fixed in 5.0.
>>
>>
>> 1655408 in contrast reports an error at startup; locking itself failed.
>>   I couldn’t reproduce it, and I still can’t; neither with the image
>> mounted concurrently, nor with an RO NFS mount.
>>
>> (For example:
>>
>> exports:
>> [...]/test-nfs-ro
>> 127.0.0.1(ro,sync,no_subtree_check,fsid=0,insecure,crossmnt)
>>
>> $ for i in $(seq 100); do \
>>  echo -e '\033[1m---\033[0m'; \
>>  x86_64-softmmu/qemu-system-x86_64 \
>>    -drive \
>>  if=none,id=drv0,readonly=on,file=/mnt/tmp/arch.iso,format=raw \
>>    -device ide-cd,drive=drv0 \
>>    -enable-kvm -m 2048 -display none &; \
>>  pid=$!; \
>>  sleep 1; \
>>  kill $pid; \
>>    done
>>
>> (Where x86_64-softmmu/qemu-system-x86_64 is upstream 5.0.1.)
>>
>> All I see is something like:
>>
>> ---
>> qemu-system-x86_64: terminating on signal 15 from pid 7278 (/bin/zsh)
>> [2] 34103
>> [3]  - 34095 terminated  x86_64-softmmu/qemu-system-x86_64 -drive
>> -device ide-cd,drive=drv0  -m 2048
>>
>> So no file locking errors.)
>>
> 
> The error I got was specifically "Failed to lock byte 100" and VM not
> starting.  The ISO file was on a R/W NFS3 share, but was itself R/O for
> the user that root was mapped to by linux-nfs-server via /etc/exports
> options, specifically the file iso file was mode 0444 in a 0755
> directory, and the exports line was (simplified)
> 
> /share1
> :::/64(ro,sync,mp,subtree_check,anonuid=1000,anongid=1000)
> 
> where :::/64 is the numeric IPv6 prefix of the LAN
> 
> NFS kernel Server ran Debian Stretch kernel 4.19.0-0.bpo.8-amd64 #1 SMP
> Debian 4.19.98-1~bpo9+1 (2020-03-09) x86_64 GNU/Linux
> 
> NFS client mount options were:
> 
> rw,nosuid,nodev,noatime,vers=3,rsize=1048576,wsize=1048576,namlen=255,
> soft,proto=tcp6,timeo=600,retrans=6,sec=sys,mountaddr=:::::xxff:fexx:,
> 
> mountvers=3,mountport=45327,mountproto=udp6,local_lock=none,addr=:::::xxff:fexx:

I’ve tried using these settings, but still can’t reproduce the bug.

Nothing else uses the image when you try to attach it to qemu, right?
(Your last email noted something about a loop mount, but I’m not sure
whether that just referred to the RH Bugzilla entry.)

(local_lock=none means that all locks are relayed to the server, correct?)

Max

[PATCH] tests/qtest/libqtest: Fix detection of architecture for binaries without path

[Thomas Huth]

The qtests can be run directly by specifying the QEMU binary with the
QTEST_QEMU_BINARY environment variable, for example:

 $ QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/qtest/test-hmp

However, if you specify a binary without a path, for example with
QTEST_QEMU_BINARY=qemu-system-x86_64 if the QEMU binary is in your
$PATH, then the test currently simply crashes.

Let's try a little bit smarter here by looking for the final '-'
instead of the slash.

Signed-off-by: Thomas Huth 
---
 tests/qtest/libqtest.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 58f58e1ece..7cbcc77fdd 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -830,9 +830,14 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...)
 const char *qtest_get_arch(void)
 {
 const char *qemu = qtest_qemu_binary();
-const char *end = strrchr(qemu, '/');
+const char *end = strrchr(qemu, '-');
 
-return end + strlen("/qemu-system-");
+if (!end) {
+fprintf(stderr, "Can't determine architecture from binary name.\n");
+abort();
+}
+
+return end + 1;
 }
 
 bool qtest_get_irq(QTestState *s, int num)
-- 
2.18.2

Re: [PATCH] hw/pci-host/grackle: Verify PIC link is properly set

[BALATON Zoltan via]

On Mon, 12 Oct 2020, David Gibson wrote:

On Mon, Oct 12, 2020 at 08:21:41AM +0200, Philippe Mathieu-Daudé wrote:

On 10/12/20 12:34 AM, David Gibson wrote:

On Sun, Oct 11, 2020 at 09:03:32PM +0200, Philippe Mathieu-Daudé wrote:

The Grackle PCI host model expects the interrupt controller
being set, but does not verify it is present. Add a check to
help developers using this model.


I don't think thaqt's very likely, but, sure, applied to ppc-for-5.2


Do you want I correct the description as:
"The Grackle PCI host model expects the interrupt controller
being set, but does not verify it is present.
A developer basing its implementation on the Grackle model
might hit this problem. Add a check to help future developers
using this model as reference."?


No, it's fine.  All I was saying is that the chances of anyone using
Grackle in future seem very low to me.


So maybe an assert instead of a user visible error would be enough?

Regards,
BALATON Zoltan










Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/pci-host/grackle.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index 57c29b20afb..20361d215ca 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -76,6 +76,10 @@ static void grackle_realize(DeviceState *dev, Error **errp)
  GrackleState *s = GRACKLE_PCI_HOST_BRIDGE(dev);
  PCIHostState *phb = PCI_HOST_BRIDGE(dev);
+if (!s->pic) {
+error_setg(errp, TYPE_GRACKLE_PCI_HOST_BRIDGE ": 'pic' link not set");
+return;
+}
  phb->bus = pci_register_root_bus(dev, NULL,
   pci_grackle_set_irq,
   pci_grackle_map_irq,

Re: [PULL 09/14] qmp: Move dispatcher to a coroutine

[Philippe Mathieu-Daudé]

On 10/12/20 1:25 PM, Kevin Wolf wrote:

Am 12.10.2020 um 12:47 hat Alex Bennée geschrieben:


Markus Armbruster  writes:


From: Kevin Wolf 

This moves the QMP dispatcher to a coroutine and runs all QMP command
handlers that declare 'coroutine': true in coroutine context so they
can avoid blocking the main loop while doing I/O or waiting for other
events.


This subtly changes the replay behaviour leading to a hang in:

   10:55:18 [alex.bennee@hackbox2:~/l/q/b/bisect] (625581c2…)|✚1(+1/-1) + 
./tests/venv/bin/avocado run 
tests/acceptance/replay_kernel.py:ReplayKernel.test_arm_virt
   Fetching asset from 
tests/acceptance/replay_kernel.py:ReplayKernel.test_arm_virt
   JOB ID : ec11fd2544f06e6c0d421f16afa757b49f7ed734
   JOB LOG: 
/home/alex.bennee/avocado/job-results/job-2020-10-12T11.40-ec11fd2/job.log
(1/1) tests/acceptance/replay_kernel.py:ReplayKernel.test_arm_virt: ERROR: 
Could not perform graceful shutdown (26.27 s)
   RESULTS: PASS 0 | ERROR 1 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
CANCEL 0
   JOB TIME   : 27.77 s

Looking at the log:

   2020-10-12 11:40:31,426 __init__ L0085 DEBUG| [3.887411] 
rtc-pl031 901.pl031: setting system clock to 2020-10-12 10:40:31 UTC 
(1602499231)
   2020-10-12 11:40:31,428 __init__ L0085 DEBUG| [3.887431] 
sr_init: No PMIC hook to init smartreflex
   2020-10-12 11:40:31,447 __init__ L0085 DEBUG| [3.897193] 
uart-pl011 900.pl011: no DMA platform data
   2020-10-12 11:40:31,460 __init__ L0085 DEBUG| [3.897242] md: 
Waiting for all devices to be available before autodetect
   2020-10-12 11:40:31,462 __init__ L0085 DEBUG| [3.897259] md: If 
you don't use raid, use raid=noautodetect
   2020-10-12 11:40:31,475 __init__ L0085 DEBUG| [3.897819] md: 
Autodetecting RAID arrays.
   2020-10-12 11:40:31,476 __init__ L0085 DEBUG| [3.897832] md: 
autorun ...
   2020-10-12 11:40:31,477 __init__ L0085 DEBUG| [3.897842] md: ... 
autorun DONE.
   2020-10-12 11:40:31,483 __init__ L0085 DEBUG| [3.897962] VFS: Cannot open 
root device "(null)" or unknown-block(0,0): error -6
   2020-10-12 11:40:31,483 qmp  L0245 DEBUG| >>> {'execute': 'quit'}
   2020-10-12 11:40:31,495 qmp  L0145 DEBUG| <<< {'timestamp': 
{'seconds': 1602499231, 'microseconds': 493379}, 'event': 'SHUTDOWN', 'data': {'guest': 
True, 'reason': 'guest-reset'}}
   2020-10-12 11:40:31,733 machine  L0325 WARNI| qemu received signal 6; command: 
"./qemu-system-arm -display none -vga none -chardev 
socket,id=mon,path=/var/tmp/tmpzls53khe/qemu-8487-monitor.sock -mon 
chardev=mon,mode=control -machine virt -chardev 
socket,id=console,path=/var/tmp/tmpzls53khe/qemu-8487-console.sock,server,nowait -serial 
chardev:console -icount 
shift=1,rr=record,rrfile=/var/tmp/avocado_n00stdrf/avocado_job_aw60qdul/1-tests_acceptance_replay_kernel.py_ReplayKernel.test_arm_virt/replay.bin
 -kernel 
/home/alex.bennee/avocado/data/cache/by_location/62750ce9e069e69e6a7ff04ff54c382ee660b92a/vmlinuz
 -append printk.time=1 panic=-1 console=ttyAMA0 -net none -no-reboot"


This looks like a crash (SIGABRT) rather than a hang. Do you have a
stack trace for the crashed process?


No crash, exit(0):

VFS: Cannot open root device "(null)" or unknown-block(0,0): error -6
Please append a correct "root=" boot option; here are the available 
partitions:
Kernel panic - not syncing: VFS: Unable to mount root fs on 
unknown-block(0,0)

CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.18.16-300.fc29.armv7hl #1
Hardware name: Generic DT based system
[] (unwind_backtrace) from [] (show_stack+0x20/0x24)
[] (show_stack) from [] (dump_stack+0x88/0xa8)
[] (dump_stack) from [] (panic+0xd4/0x26c)
[] (panic) from [] (mount_block_root+0x250/0x2ec)
[] (mount_block_root) from [] (mount_root+0x78/0x90)
[] (mount_root) from [] (prepare_namespace+0x15c/0x19c)
[] (prepare_namespace) from [] 
(kernel_init_freeable+0x2c0/0x370)
[] (kernel_init_freeable) from [] 
(kernel_init+0x18/0x128)

[] (kernel_init) from [] (ret_from_fork+0x14/0x2c)
Exception stack(0xc790bfb0 to 0xc790bff8)
bfa0:    

bfc0:        


bfe0:     0013 

-> PSCI call
   -> QEMU_PSCI_0_2_FN_SYSTEM_RESET
  -> SHUTDOWN_CAUSE_GUEST_RESET
 -> exit(0)



Kevin


   2020-10-12 11:40:31,734 stacktrace   L0039 ERROR|
   2020-10-12 11:40:31,734 stacktrace   L0042 ERROR| Reproduced traceback 
from: 
/home/alex.bennee/lsrc/qemu.git/builds/bisect/tests/venv/lib/python3.6/site-packages/avocado/core/test.py:767
   2020-10-12 11:40:31,735 stacktrace   L0045 ERROR| Traceback (most recent 
call last):
   2020-10-12 11:40:31,735 stacktrace   L0045 ERROR|   File 
"/home/alex.bennee/lsrc/qemu.git/python/qemu/machine.py", line 435, in 
_do_shutdown
   2020-10-12 11:40:31,735 s

Re: [PATCH v4 4/7] keyval: Parse help options

[Eric Blake]

On 10/11/20 2:35 AM, Markus Armbruster wrote:

From: Kevin Wolf 

This adds a special meaning for 'help' and '?' as options to the keyval
parser. Instead of being an error (because of a missing value) or a
value for an implied key, they now request help, which is a new boolean
output of the parser in addition to the QDict.

A new parameter 'p_help' is added to keyval_parse() that contains on
return whether help was requested. If NULL is passed, requesting help
results in an error and all other cases work like before.




+
+/* "help" by itself, without implied key */
+qdict = keyval_parse("help", NULL, &help, &error_abort);
+g_assert_cmpuint(qdict_size(qdict), ==, 0);
+g_assert(help);
+qobject_unref(qdict);
+
+/* "help" by itself, with implied key */
+qdict = keyval_parse("help", "implied", &help, &error_abort);
+g_assert_cmpuint(qdict_size(qdict), ==, 0);
+g_assert(help);
+qobject_unref(qdict);
+
+/* "help" when no help is available, without implied key */
+qdict = keyval_parse("help", NULL, NULL, &err);
+error_free_or_abort(&err);
+g_assert(!qdict);
+
+/* "help" when no help is available, with implied key */
+qdict = keyval_parse("help", "implied", NULL, &err);
+error_free_or_abort(&err);
+g_assert(!qdict);
+
+/* Key "help" */
+qdict = keyval_parse("help=on", NULL, &help, &error_abort);
+g_assert_cmpuint(qdict_size(qdict), ==, 1);
+g_assert_cmpstr(qdict_get_try_str(qdict, "help"), ==, "on");
+g_assert(!help);
+qobject_unref(qdict);
+
+/* "help" followed by crap, without implied key */
+qdict = keyval_parse("help.abc", NULL, &help, &err);
+error_free_or_abort(&err);
+g_assert(!qdict);
+
+/* "help" followed by crap, with implied key */
+qdict = keyval_parse("help.abc", "implied", &help, &err);
+g_assert_cmpuint(qdict_size(qdict), ==, 1);
+g_assert_cmpstr(qdict_get_try_str(qdict, "implied"), ==, "help.abc");
+g_assert(!help);
+qobject_unref(qdict);
+
+/* "help" with other stuff, without implied key */
+qdict = keyval_parse("number=42,help,foo=bar", NULL, &help, &error_abort);
+g_assert_cmpuint(qdict_size(qdict), ==, 2);
+g_assert_cmpstr(qdict_get_try_str(qdict, "number"), ==, "42");
+g_assert_cmpstr(qdict_get_try_str(qdict, "foo"), ==, "bar");
+g_assert(help);
+qobject_unref(qdict);
+
+/* "help" with other stuff, with implied key */
+qdict = keyval_parse("val,help,foo=bar", "implied", &help, &error_abort);
+g_assert_cmpuint(qdict_size(qdict), ==, 2);
+g_assert_cmpstr(qdict_get_try_str(qdict, "implied"), ==, "val");
+g_assert_cmpstr(qdict_get_try_str(qdict, "foo"), ==, "bar");
+g_assert(help);
+qobject_unref(qdict);


Is it worth checking that "helper" with implied key is a value, not help?


+++ b/util/keyval.c
@@ -14,10 +14,11 @@
   * KEY=VALUE,... syntax:
   *
   *   key-vals = [ key-val { ',' key-val } [ ',' ] ]
- *   key-val  = key '=' val
+ *   key-val  = key '=' val | help
   *   key  = key-fragment { '.' key-fragment }
   *   key-fragment = / [^=,.]+ /
   *   val  = { / [^,]+ / | ',,' }
+ *   help = 'help | '?'


Missing '

Otherwise
Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[PULL 2/6] tools/virtiofsd: add support for --socket-group

[Dr. David Alan Gilbert (git)]

From: Alex Bennée 

If you like running QEMU as a normal user (very common for TCG runs)
but you have to run virtiofsd as a root user you run into connection
problems. Adding support for an optional --socket-group allows the
users to keep using the command line.

Signed-off-by: Alex Bennée 
Reviewed-by: Stefan Hajnoczi 

Message-Id: <20200925125147.26943-2-alex.ben...@linaro.org>
Signed-off-by: Dr. David Alan Gilbert 
  dgilbert: Split long line
---
 docs/tools/virtiofsd.rst|  4 
 tools/virtiofsd/fuse_i.h|  1 +
 tools/virtiofsd/fuse_lowlevel.c |  6 ++
 tools/virtiofsd/fuse_virtio.c   | 21 +++--
 4 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
index ae02938a95..7ecee49834 100644
--- a/docs/tools/virtiofsd.rst
+++ b/docs/tools/virtiofsd.rst
@@ -87,6 +87,10 @@ Options
 
   Listen on vhost-user UNIX domain socket at PATH.
 
+.. option:: --socket-group=GROUP
+
+  Set the vhost-user UNIX domain socket gid to GROUP.
+
 .. option:: --fd=FDNUM
 
   Accept connections from vhost-user UNIX domain socket file descriptor FDNUM.
diff --git a/tools/virtiofsd/fuse_i.h b/tools/virtiofsd/fuse_i.h
index 1240828208..492e002181 100644
--- a/tools/virtiofsd/fuse_i.h
+++ b/tools/virtiofsd/fuse_i.h
@@ -68,6 +68,7 @@ struct fuse_session {
 size_t bufsize;
 int error;
 char *vu_socket_path;
+char *vu_socket_group;
 int   vu_listen_fd;
 int   vu_socketfd;
 struct fv_VuDev *virtio_dev;
diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index 2dd36ec03b..4d1ba2925d 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -2523,6 +2523,7 @@ static const struct fuse_opt fuse_ll_opts[] = {
 LL_OPTION("--debug", debug, 1),
 LL_OPTION("allow_root", deny_others, 1),
 LL_OPTION("--socket-path=%s", vu_socket_path, 0),
+LL_OPTION("--socket-group=%s", vu_socket_group, 0),
 LL_OPTION("--fd=%d", vu_listen_fd, 0),
 LL_OPTION("--thread-pool-size=%d", thread_pool_size, 0),
 FUSE_OPT_END
@@ -2630,6 +2631,11 @@ struct fuse_session *fuse_session_new(struct fuse_args 
*args,
  "fuse: --socket-path and --fd cannot be given together\n");
 goto out4;
 }
+if (se->vu_socket_group && !se->vu_socket_path) {
+fuse_log(FUSE_LOG_ERR,
+ "fuse: --socket-group can only be used with --socket-path\n");
+goto out4;
+}
 
 se->bufsize = FUSE_MAX_MAX_PAGES * getpagesize() + FUSE_BUFFER_HEADER_SIZE;
 
diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
index d5c8e98253..89f537f79b 100644
--- a/tools/virtiofsd/fuse_virtio.c
+++ b/tools/virtiofsd/fuse_virtio.c
@@ -31,6 +31,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
 #include "contrib/libvhost-user/libvhost-user.h"
@@ -924,15 +926,30 @@ static int fv_create_listen_socket(struct fuse_session 
*se)
 
 /*
  * Unfortunately bind doesn't let you set the mask on the socket,
- * so set umask to 077 and restore it later.
+ * so set umask appropriately and restore it later.
  */
-old_umask = umask(0077);
+if (se->vu_socket_group) {
+old_umask = umask(S_IROTH | S_IWOTH | S_IXOTH);
+} else {
+old_umask = umask(S_IRGRP | S_IWGRP | S_IXGRP |
+  S_IROTH | S_IWOTH | S_IXOTH);
+}
 if (bind(listen_sock, (struct sockaddr *)&un, addr_len) == -1) {
 fuse_log(FUSE_LOG_ERR, "vhost socket bind: %m\n");
 close(listen_sock);
 umask(old_umask);
 return -1;
 }
+if (se->vu_socket_group) {
+struct group *g = getgrnam(se->vu_socket_group);
+if (g) {
+if (!chown(se->vu_socket_path, -1, g->gr_gid)) {
+fuse_log(FUSE_LOG_WARNING,
+ "vhost socket failed to set group to %s (%d)\n",
+ se->vu_socket_group, g->gr_gid);
+}
+}
+}
 umask(old_umask);
 
 if (listen(listen_sock, 1) == -1) {
-- 
2.28.0

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

[Greg Kurz]

On Mon, 12 Oct 2020 13:40:33 +0200
BALATON Zoltan via  wrote:

> On Mon, 12 Oct 2020, Alexey Kardashevskiy wrote:
> > On 29/09/2020 20:35, Alexey Kardashevskiy wrote:
> >> 
> >> On 16/07/2020 23:22, David Gibson wrote:
> >>> On Thu, Jul 16, 2020 at 07:04:56PM +1000, Alexey Kardashevskiy wrote:
>  Ping? I kinda realize it is not going to replace SLOF any time soon but
>  still...
> >>> 
> >>> Yeah, I know.   I just haven't had time to consider it.  Priority
> >>> starvation.
> >> 
> >> 
> >> Still? :)
> >
> > Ping?
> 
> +1, I'd like to see this merged and experiment with it to emulate firmware 
> for pegasos2 but I'd rather use the final version than something off-tree 
> which may end up different when gets upstream. Is there a way I could help 
> with this?
> 

This patch is a bit _old_ ;) I haven't checked the details but it might
need some rebasing. Especially it should be ported to using meson if
someone wants to experiment with it.

> Regards,
> BALATON Zoltan