Re: [PATCH 18/19] target/ppc/pmu_book3s_helper.c: add PM_CMPLU_STALL mock events

[David Gibson]

On Tue, Aug 10, 2021 at 04:48:59PM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 8/10/21 1:17 AM, David Gibson wrote:
> > On Mon, Aug 09, 2021 at 10:10:56AM -0300, Daniel Henrique Barboza wrote:
> > > EBB powerpc kernel test 'multi_counter_test' uses PM_CMPLU_STALL events
> > > that we do not support. These events are related to CPU stalled/wasted
> > > cycles while waiting for resources, cache misses and so on.
> > > 
> > > Unlike the 0xFA event added previously, there's no available equivalent
> > > for us to use, and at this moment we can't sample those events as well.
> > > What we can do is mock those events as if we were calculating them.
> > > 
> > > This patch implements PM_CMPLU_STALL, PM_CMPLU_STALL_FXU,
> > > PM_CMPLU_STALL_OTHER_CMPL and PM_CMPLU_STALL_THRD mock events by giving
> > > them a fixed amount of the total elapsed cycles.
> > > 
> > > The chosen sample values for these events (25% of total cycles for
> > > PM_CMPLU_STALL and 5% for the other three) were chosen at random and has
> > > no intention of being truthful with what a real PowerPC hardware would
> > > give us. Our intention here is to make 'multi_counter_test' EBB test
> > > pass.
> > 
> > Hmm.  I guess these mock values make sense for getting the kernel
> > tests to pass, but I'm not sure if it's a good idea in general.  Would
> > we be better off just reporting 0 always - that would be a strong hint
> > to someone attempting to analyze results that something is fishy (in
> > this case that they don't actually have a real CPU).
> 
> We can drop this patch and I'll add a note in the docs that the EBB kernel
> test 'multi_counter_test' fails because the PMU does not implement STALL
> events.

Sounds good for now.  We can reconsider this if we have a specific
justification for it.

-- 
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 12/19] target/ppc/pmu_book3s_helper.c: enable PMC1 counter negative EBB

[David Gibson]

On Tue, Aug 10, 2021 at 05:26:09PM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 8/10/21 1:01 AM, David Gibson wrote:
> > On Mon, Aug 09, 2021 at 10:10:50AM -0300, Daniel Henrique Barboza wrote:
> > > This patch starts the counter negative EBB support by enabling PMC1
> > > counter negative condition.
> > > 
> > > A counter negative condition happens when a performance monitor counter
> > > reaches the value 0x8000. When that happens, if a counter negative
> > > condition is enabled in that counter, a performance monitor alert is
> > > triggered. For PMC1, this condition is enabled by MMCR0_PMC1CE.
> > > 
> > > An icount-based logic is used to predict when we need to wake up the timer
> > > to trigger the alert in both PM_INST_CMPL (0x2) and PM_CYC (0x1E) events.
> > > The timer callback will then trigger a PPC_INTERRUPT_PMC which will 
> > > become a
> > > event-based exception later.
> > > 
> > > Some EBB powerpc kernel selftests are passing after this patch, but a
> > > substancial amount of them relies on other PMCs to be enabled and events
> > > that we don't support at this moment. We'll address that in the next
> > > patches.
> > > 
> > > Signed-off-by: Daniel Henrique Barboza 
> > > ---
> > >   target/ppc/cpu.h   |   1 +
> > >   target/ppc/pmu_book3s_helper.c | 127 +++--
> > >   2 files changed, 92 insertions(+), 36 deletions(-)
> > > 
> > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > > index 1d38b8cf7a..5c81d459f4 100644
> > > --- a/target/ppc/cpu.h
> > > +++ b/target/ppc/cpu.h
> > > @@ -350,6 +350,7 @@ typedef struct ppc_v3_pate_t {
> > >   #define MMCR0_EBE   PPC_BIT(43) /* Perf Monitor EBB Enable */
> > >   #define MMCR0_FCECE PPC_BIT(38) /* FC on Enabled Cond or Event 
> > > */
> > >   #define MMCR0_PMCC  PPC_BITMASK(44, 45) /* PMC Control */
> > > +#define MMCR0_PMC1CE PPC_BIT(48)
> > >   #define MMCR1_PMC1SEL_SHIFT (63 - 39)
> > >   #define MMCR1_PMC1SEL PPC_BITMASK(32, 39)
> > > diff --git a/target/ppc/pmu_book3s_helper.c 
> > > b/target/ppc/pmu_book3s_helper.c
> > > index 43cc0eb722..58ae65e22b 100644
> > > --- a/target/ppc/pmu_book3s_helper.c
> > > +++ b/target/ppc/pmu_book3s_helper.c
> > > @@ -25,6 +25,7 @@
> > >* and SPAPR code.
> > >*/
> > >   #define PPC_CPU_FREQ 10
> > > +#define COUNTER_NEGATIVE_VAL 0x8000
> > >   static uint64_t get_cycles(uint64_t icount_delta)
> > >   {
> > > @@ -32,22 +33,9 @@ static uint64_t get_cycles(uint64_t icount_delta)
> > >   NANOSECONDS_PER_SECOND);
> > >   }
> > > -static void update_PMC_PM_INST_CMPL(CPUPPCState *env, int sprn,
> > > -uint64_t icount_delta)
> > > -{
> > > -env->spr[sprn] += icount_delta;
> > > -}
> > > -
> > > -static void update_PMC_PM_CYC(CPUPPCState *env, int sprn,
> > > -  uint64_t icount_delta)
> > > -{
> > > -env->spr[sprn] += get_cycles(icount_delta);
> > > -}
> > > -
> > > -static void update_programmable_PMC_reg(CPUPPCState *env, int sprn,
> > > -uint64_t icount_delta)
> > > +static uint8_t get_PMC_event(CPUPPCState *env, int sprn)
> > >   {
> > > -int event;
> > > +int event = 0x0;
> > >   switch (sprn) {
> > >   case SPR_POWER_PMC1:
> > > @@ -65,11 +53,35 @@ static void update_programmable_PMC_reg(CPUPPCState 
> > > *env, int sprn,
> > >   case SPR_POWER_PMC4:
> > >   event = MMCR1_PMC4SEL & env->spr[SPR_POWER_MMCR1];
> > >   break;
> > > +case SPR_POWER_PMC5:
> > > +event = 0x2;
> > > +break;
> > > +case SPR_POWER_PMC6:
> > > +event = 0x1E;
> > > +break;
> > 
> > This looks like a nice cleanup that would be better folded into an
> > earlier patch.
> > 
> > >   default:
> > > -return;
> > > +break;
> > >   }
> > > -switch (event) {
> > > +return event;
> > > +}
> > > +
> > > +static void update_PMC_PM_INST_CMPL(CPUPPCState *env, int sprn,
> > > +uint64_t icount_delta)
> > > +{
> > > +env->spr[sprn] += icount_delta;
> > > +}
> > > +
> > > +static void update_PMC_PM_CYC(CPUPPCState *env, int sprn,
> > > +  uint64_t icount_delta)
> > > +{
> > > +env->spr[sprn] += get_cycles(icount_delta);
> > > +}
> > > +
> > > +static void update_programmable_PMC_reg(CPUPPCState *env, int sprn,
> > > +uint64_t icount_delta)
> > > +{
> > > +switch (get_PMC_event(env, sprn)) {
> > >   case 0x2:
> > >   update_PMC_PM_INST_CMPL(env, sprn, icount_delta);
> > >   break;
> > > @@ -99,30 +111,57 @@ static void update_PMCs(CPUPPCState *env, uint64_t 
> > > icount_delta)
> > >   update_PMC_PM_CYC(env, SPR_POWER_PMC6, icount_delta);
> > >   }
> > > +static void set_PMU_excp_timer(CPUPPCState *env)
> > > +{
> > > +uint64_t timeout, now, remaining_val;
> > > +
> > > +if (!(env->spr[SPR

Re: [PATCH v4] failover: unregister ROM on unplug

[Michael S. Tsirkin]

On Wed, Jul 21, 2021 at 06:09:05PM +0200, Laurent Vivier wrote:
> The intend of failover is to allow to migrate a VM with a VFIO
> networking card without disrupting the network operation by switching
> to a virtio-net device during the migration.
> 
> This simple change allows to test failover with a simulated device
> like e1000e rather than a vfio device, even if it's useless in real
> life it can help to debug failover.
> 
> This is interesting to developers that want to test failover on
> a system with no vfio device. Moreover it simplifies host networking
> configuration as we can use the same bridge for virtio-net and
> the other failover networking device.
> 
> Without this change the migration of a system configured with failover
> fails with:
> 
>   ...
>   -device virtio-net-pci,id=virtionet0,failover=on,...  \
>   -device e1000,failover_pair_id=virtionet0,... \
>   ...
> 
>   (qemu) migrate ...
> 
>   Unknown ramblock ":00:01.1:00.0/e1000e.rom", cannot accept migration
>   error while loading state for instance 0x0 of device 'ram'
>   load of migration failed: Invalid argument
> 
> This happens because QEMU correctly unregisters the interface vmstate but
> not the ROM one. This patch fixes that.
> 
> Signed-off-by: Laurent Vivier 

I began to wonder about this. Why are we sending the option ROM at all?
I think there's no need to do it for the primary ...

> ---
> 
> Notes:
> v4:
>   export and use pci_del_option_rom()
> 
> v3:
>   remove useless space before comma
> 
> v2:
>   reset has_rom to false
>   update commit log message
> 
>  include/hw/pci/pci.h | 2 ++
>  hw/net/virtio-net.c  | 1 +
>  hw/pci/pci.c | 3 +--
>  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index d0f4266e3725..84707034cbf8 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -369,6 +369,8 @@ void pci_register_vga(PCIDevice *pci_dev, MemoryRegion 
> *mem,
>  void pci_unregister_vga(PCIDevice *pci_dev);
>  pcibus_t pci_get_bar_addr(PCIDevice *pci_dev, int region_num);
>  
> +void pci_del_option_rom(PCIDevice *pdev);
> +
>  int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
> uint8_t offset, uint8_t size,
> Error **errp);
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 16d20cdee52a..d6f03633f1b3 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -3256,6 +3256,7 @@ static void 
> virtio_net_handle_migration_primary(VirtIONet *n, MigrationState *s)
>  if (migration_in_setup(s) && !should_be_hidden) {
>  if (failover_unplug_primary(n, dev)) {
>  vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev);
> +pci_del_option_rom(PCI_DEVICE(dev));
>  qapi_event_send_unplug_primary(dev->id);
>  qatomic_set(&n->failover_primary_hidden, true);
>  } else {
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 23d2ae2ab232..c210d92b5ba7 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -228,7 +228,6 @@ static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
>  static void pci_update_mappings(PCIDevice *d);
>  static void pci_irq_handler(void *opaque, int irq_num, int level);
>  static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom, Error 
> **);
> -static void pci_del_option_rom(PCIDevice *pdev);
>  
>  static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;
>  static uint16_t pci_default_sub_device_id = PCI_SUBDEVICE_ID_QEMU;
> @@ -2429,7 +2428,7 @@ static void pci_add_option_rom(PCIDevice *pdev, bool 
> is_default_rom,
>  pci_register_bar(pdev, PCI_ROM_SLOT, 0, &pdev->rom);
>  }
>  
> -static void pci_del_option_rom(PCIDevice *pdev)
> +void pci_del_option_rom(PCIDevice *pdev)
>  {
>  if (!pdev->has_rom)
>  return;
> -- 
> 2.31.1

[PULL 7/7] MAINTAINERS: update virtio-gpu entry.

[Gerd Hoffmann]

New maintainer wanted. Downgrade status to "Odd Fixes" for now.

Signed-off-by: Gerd Hoffmann 
Message-Id: <20210810083450.2377374-8-kra...@redhat.com>
---
 MAINTAINERS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6e86426572a3..6b3697962c1b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2157,7 +2157,7 @@ F: include/hw/display/ramfb.h
 
 virtio-gpu
 M: Gerd Hoffmann 
-S: Maintained
+S: Odd Fixes
 F: hw/display/virtio-gpu*
 F: hw/display/virtio-vga.*
 F: include/hw/virtio/virtio-gpu.h
@@ -2176,7 +2176,7 @@ F: include/hw/virtio/vhost-user-scsi.h
 
 vhost-user-gpu
 M: Marc-André Lureau 
-M: Gerd Hoffmann 
+R: Gerd Hoffmann 
 S: Maintained
 F: docs/interop/vhost-user-gpu.rst
 F: contrib/vhost-user-gpu
-- 
2.31.1

Re: [PATCH 13/19] target/ppc/translate: PMU: handle setting of PMCs while running

[David Gibson]

On Tue, Aug 10, 2021 at 05:44:18PM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 8/10/21 1:06 AM, David Gibson wrote:
> > On Mon, Aug 09, 2021 at 10:10:51AM -0300, Daniel Henrique Barboza wrote:
> > > The initial PMU support were made under the assumption that the counters
> > > would be set before running the PMU and read after either freezing the
> > > PMU manually or via a performance monitor alert.
> > > 
> > > Turns out that some EBB powerpc kernel tests set the counters after
> > > unfreezing the counters. Setting a PMC value when the PMU is running
> > > means that, at that moment, the baseline for calculating the events (set
> > > in env->pmu_base_icount) needs to be updated. Updating this baseline
> > > means that we need to update all the PMCs with their actual value at
> > > that moment. Any existing counter negative timer needs to be discarded
> > > an a new one, with the updated values, must be set again.
> > > 
> > > This patch does that via a new 'helper_store_pmc()' that is called in
> > > the mtspr() callbacks of the PMU registers, spr_write_pmu_ureg() and
> > > spr_write_pmu_generic() in target/ppc/translate.c
> > > 
> > > With this change, EBB powerpc kernel tests such as  'no_handler_test'
> > > are now passing.
> > > 
> > > Signed-off-by: Daniel Henrique Barboza 
> > > ---
> > >   target/ppc/helper.h|  1 +
> > >   target/ppc/pmu_book3s_helper.c | 36 --
> > >   target/ppc/translate.c | 27 +
> > >   3 files changed, 62 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> > > index 5122632784..757665b360 100644
> > > --- a/target/ppc/helper.h
> > > +++ b/target/ppc/helper.h
> > > @@ -21,6 +21,7 @@ DEF_HELPER_1(hrfid, void, env)
> > >   DEF_HELPER_2(store_lpcr, void, env, tl)
> > >   DEF_HELPER_2(store_pcr, void, env, tl)
> > >   DEF_HELPER_2(store_mmcr0, void, env, tl)
> > > +DEF_HELPER_3(store_pmc, void, env, i32, i64)
> > >   #endif
> > >   DEF_HELPER_1(check_tlb_flush_local, void, env)
> > >   DEF_HELPER_1(check_tlb_flush_global, void, env)
> > > diff --git a/target/ppc/pmu_book3s_helper.c 
> > > b/target/ppc/pmu_book3s_helper.c
> > > index 58ae65e22b..e7af273cb6 100644
> > > --- a/target/ppc/pmu_book3s_helper.c
> > > +++ b/target/ppc/pmu_book3s_helper.c
> > > @@ -173,7 +173,7 @@ void cpu_ppc_pmu_timer_init(CPUPPCState *env)
> > >   env->pmu_intr_timer = timer;
> > >   }
> > > -static bool mmcr0_counter_neg_cond_enabled(uint64_t mmcr0)
> > > +static bool counter_negative_cond_enabled(uint64_t mmcr0)
> > 
> > Can you fold this rename into the patch which introduces the function
> > please.
> > 
> > >   {
> > >   return mmcr0 & MMCR0_PMC1CE;
> > >   }
> > > @@ -219,9 +219,41 @@ void helper_store_mmcr0(CPUPPCState *env, 
> > > target_ulong value)
> > >* Start performance monitor alert timer for counter 
> > > negative
> > >* events, if needed.
> > >*/
> > > -if 
> > > (mmcr0_counter_neg_cond_enabled(env->spr[SPR_POWER_MMCR0])) {
> > > +if 
> > > (counter_negative_cond_enabled(env->spr[SPR_POWER_MMCR0])) {
> > >   set_PMU_excp_timer(env);
> > >   }
> > >   }
> > >   }
> > >   }
> > > +
> > > +void helper_store_pmc(CPUPPCState *env, uint32_t sprn, uint64_t value)
> > > +{
> > > +bool pmu_frozen = env->spr[SPR_POWER_MMCR0] & MMCR0_FC;
> > > +uint64_t curr_icount, icount_delta;
> > > +
> > > +if (pmu_frozen) {
> > > +env->spr[sprn] = value;
> > > +return;
> > > +}
> > > +
> > > +curr_icount = (uint64_t)icount_get_raw();
> > > +icount_delta = curr_icount - env->pmu_base_icount;
> > > +
> > > +/* Update the counter with the events counted so far */
> > > +update_PMCs(env, icount_delta);
> > > +
> > > +/* Set the counter to the desirable value after update_PMCs() */
> > > +env->spr[sprn] = value;
> > > +
> > > +/*
> > > + * Delete the current timer and restart a new one with the
> > > + * updated values.
> > > + */
> > > +timer_del(env->pmu_intr_timer);
> > > +
> > > +env->pmu_base_icount = curr_icount;
> > 
> > I'd expect some of this code to be shared with the unfreeze path using
> > a helper.  Is there a reason that's not the case?
> > 
> > Do you also need to deal with any counter interrupts that have already
> > been generated by the old counter?  Are the counter overflow events
> > edge-triggered or level-triggered?
> 
> 
> I'd say that counter negative overflow are edge triggered - we can set any
> starting value for the counters but the counter negative overflow is
> triggered always with the counter reaching 0x800.
> 
> If a counter was about to trigger a c.n overflow and then the user set it
> back to 0, that c.n overflow is lost.

Hm.. those aren't actually the edge cases I'm unsure about.  The ones
I'm unclear on are:

1) If you directly set the PMC to a negat

[PULL 5/7] MAINTAINERS: update usb entries.

[Gerd Hoffmann]

New maintainer wanted.  Switch role to "Reviewer" for usb-serial,
downgrade status to "Odd Fixes" for everything else.

Cc: Samuel Thibault 
Signed-off-by: Gerd Hoffmann 
Message-Id: <20210810083450.2377374-6-kra...@redhat.com>
---
 MAINTAINERS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 33b4fc25e349..b84b7e33e4d0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1832,7 +1832,7 @@ F: tests/qtest/sdhci-test.c
 
 USB
 M: Gerd Hoffmann 
-S: Maintained
+S: Odd Fixes
 F: hw/usb/*
 F: stubs/usb-dev-stub.c
 F: tests/qtest/usb-*-test.c
@@ -1841,7 +1841,7 @@ F: include/hw/usb.h
 F: include/hw/usb/
 
 USB (serial adapter)
-M: Gerd Hoffmann 
+R: Gerd Hoffmann 
 M: Samuel Thibault 
 S: Maintained
 F: hw/usb/dev-serial.c
-- 
2.31.1

Re: [PATCH 06/19] target/ppc/pmu_book3s_helper: enable PMC1-PMC4 events

[David Gibson]

On Tue, Aug 10, 2021 at 12:03:15PM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 8/10/21 12:42 AM, David Gibson wrote:
> > On Mon, Aug 09, 2021 at 10:10:44AM -0300, Daniel Henrique Barboza wrote:
> > > So far the PMU logic was using PMC5 for instruction counting (linux
> > > kernel PM_INST_CMPL) and PMC6 to count cycles (PM_CYC). We aren't using
> > > PMCs 1-4.
> > > 
> > > Let's enable all PMCs to count these 2 events we already provide. The
> > > logic used to calculate PMC5 is now being provided by
> > > update_PMC_PM_INST_CMPL() and PMC6 logic is now implemented in
> > > update_PMC_PM_CYC().
> > > 
> > > The enablement of these 2 events for all PMUs are done by using the
> > > Linux kernel definition of those events: 0x02 for PM_INST_CMPL and 0x1e
> > > for PM_CYC,
> > 
> > I'm confused by this.  Surely the specific values here should be
> > defined by the hardware, not by Linux.
> 
> The hardware/PowerISA defines these events as follows for all counters:
> 
> 00 Disable events. (No events occur.)
> 01-BF Implementation-dependent
> C0-DF Reserved
> 
> And then hardware events defined by the ISA goes from E0 to FF. Each counter
> has a different set of hardware defined events in this range.
> 
> The Linux perf driver defines some events in the 'Implementation-dependent'
> area, allowing for events codes such as '0x02' to count instructions
> completed in PMC1 even though this event is not defined in the ISA for this
> PMC. I am assuming that the real hardware - at least the ones that IBM
> produces - does this mapping internally. I'll ask around to see if I find
> out whether it's the HW or some part of the Perf subsystem that I don't
> comprehend that are doing it.
> 
> 
> I am not particularly happy about having to rely on 'implementation-dependent'
> fields that are defined by the Perf subsystem of Linux in the emulator
> code that should be OS-agnostic. Unfortunately, I didn't find any alternative
> to make the kernel operate this PMU implementation other than baking these
> event codes into the PMU logic.

Ok, so they are hardware defined, just not architecture defined,
IIUC.  I think that just needs a change to the description - and
making it more explicit that this is the power9 (?) PMU you're
modelling specifically not the (barely) architected "book3s" PMU.

> 
> 
> Thanks,
> 
> 
> Daniel
> 
> 
> > 
> > > all of those defined by specific bits in MMCR1 for each PMC.
> > > PMCs 1-4 relies on the correct event to be defined in MMCR1. PMC5 and
> > > PMC6 will count PM_INST_CMPL and PMC_CYC, respectively, regardless of
> > > MMCR1 setup.
> > > 
> > > Signed-off-by: Daniel Henrique Barboza 
> > > ---
> > >   target/ppc/cpu.h   |  8 +
> > >   target/ppc/pmu_book3s_helper.c | 60 --
> > >   2 files changed, 65 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > > index 8cea8f2aca..afd9cd402b 100644
> > > --- a/target/ppc/cpu.h
> > > +++ b/target/ppc/cpu.h
> > > @@ -350,6 +350,14 @@ typedef struct ppc_v3_pate_t {
> > >   #define MMCR0_FCECE PPC_BIT(38) /* FC on Enabled Cond or Event 
> > > */
> > >   #define MMCR0_PMCC  PPC_BITMASK(44, 45) /* PMC Control */
> > > +#define MMCR1_PMC1SEL_SHIFT (63 - 39)
> > > +#define MMCR1_PMC1SEL PPC_BITMASK(32, 39)
> > > +#define MMCR1_PMC2SEL_SHIFT (63 - 47)
> > > +#define MMCR1_PMC2SEL PPC_BITMASK(40, 47)
> > > +#define MMCR1_PMC3SEL_SHIFT (63 - 55)
> > > +#define MMCR1_PMC3SEL PPC_BITMASK(48, 55)
> > > +#define MMCR1_PMC4SEL PPC_BITMASK(56, 63)
> > > +
> > >   /* LPCR bits */
> > >   #define LPCR_VPM0 PPC_BIT(0)
> > >   #define LPCR_VPM1 PPC_BIT(1)
> > > diff --git a/target/ppc/pmu_book3s_helper.c 
> > > b/target/ppc/pmu_book3s_helper.c
> > > index 0994531f65..99e62bd37b 100644
> > > --- a/target/ppc/pmu_book3s_helper.c
> > > +++ b/target/ppc/pmu_book3s_helper.c
> > > @@ -28,6 +28,56 @@ static uint64_t get_cycles(uint64_t insns)
> > >   return insns * 4;
> > >   }
> > > +static void update_PMC_PM_INST_CMPL(CPUPPCState *env, int sprn,
> > > +uint64_t curr_icount)
> > > +{
> > > +env->spr[sprn] += curr_icount - env->pmu_base_icount;
> > > +}
> > > +
> > > +static void update_PMC_PM_CYC(CPUPPCState *env, int sprn,
> > > +  uint64_t curr_icount)
> > > +{
> > > +uint64_t insns = curr_icount - env->pmu_base_icount;
> > > +env->spr[sprn] += get_cycles(insns);
> > > +}
> > > +
> > > +static void update_programmable_PMC_reg(CPUPPCState *env, int sprn,
> > > +uint64_t curr_icount)
> > > +{
> > > +int event;
> > > +
> > > +switch (sprn) {
> > > +case SPR_POWER_PMC1:
> > > +event = MMCR1_PMC1SEL & env->spr[SPR_POWER_MMCR1];
> > > +event = event >> MMCR1_PMC1SEL_SHIFT;
> > > +break;
> > > +case SPR_POWER_PMC2:
> > > +event = MMCR1_PMC2SEL & env->spr[SPR_POWER_MMCR1];
> > > +event = event >> 

[PULL 3/7] MAINTAINERS: update audio entry.

[Gerd Hoffmann]

New maintainer wanted. Downgrade status to "Odd Fixes" for now.

Signed-off-by: Gerd Hoffmann 
Message-Id: <20210810083450.2377374-4-kra...@redhat.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 79b0148cfc27..5cb402d402ec 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2258,7 +2258,7 @@ Subsystems
 --
 Audio
 M: Gerd Hoffmann 
-S: Maintained
+S: Odd Fixes
 F: audio/
 F: hw/audio/
 F: include/hw/audio/
-- 
2.31.1

[PULL 1/7] MAINTAINERS: update edk2 entry.

[Gerd Hoffmann]

I want keep an eye on the edk2 things happening in qemu.

Cc: Philippe Mathieu-Daudé 
Signed-off-by: Gerd Hoffmann 
Message-Id: <20210810083450.2377374-2-kra...@redhat.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 694973ed230d..d6c84bd0a983 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2947,6 +2947,7 @@ F: docs/interop/firmware.json
 
 EDK2 Firmware
 M: Philippe Mathieu-Daudé 
+R: Gerd Hoffmann 
 S: Supported
 F: hw/i386/*ovmf*
 F: pc-bios/descriptors/??-edk2-*.json
-- 
2.31.1

[PULL 6/7] MAINTAINERS: update virtio-input entry.

[Gerd Hoffmann]

New maintainer wanted. Downgrade status to "Odd Fixes" for now.

Signed-off-by: Gerd Hoffmann 
Message-Id: <20210810083450.2377374-7-kra...@redhat.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index b84b7e33e4d0..6e86426572a3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1951,7 +1951,7 @@ L: virtio...@redhat.com
 
 virtio-input
 M: Gerd Hoffmann 
-S: Maintained
+S: Odd Fixes
 F: hw/input/vhost-user-input.c
 F: hw/input/virtio-input*.c
 F: include/hw/virtio/virtio-input.h
-- 
2.31.1

[PULL 2/7] MAINTAINERS: update sockets entry.

[Gerd Hoffmann]

I have not touched the code for years.
Make the entry match reality and drop my name.

Cc: Daniel P. Berrange 
Signed-off-by: Gerd Hoffmann 
Message-Id: <20210810083450.2377374-3-kra...@redhat.com>
---
 MAINTAINERS | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index d6c84bd0a983..79b0148cfc27 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2843,7 +2843,6 @@ F: tests/unit/test-authz-*
 
 Sockets
 M: Daniel P. Berrange 
-M: Gerd Hoffmann 
 S: Maintained
 F: include/qemu/sockets.h
 F: util/qemu-sockets.c
-- 
2.31.1

[PULL 4/7] MAINTAINERS: update spice entry.

[Gerd Hoffmann]

New maintainer wanted. Downgrade status to "Odd Fixes" for now.

Signed-off-by: Gerd Hoffmann 
Message-Id: <20210810083450.2377374-5-kra...@redhat.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5cb402d402ec..33b4fc25e349 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2470,7 +2470,7 @@ F: scripts/coccinelle/memory-region-housekeeping.cocci
 
 SPICE
 M: Gerd Hoffmann 
-S: Supported
+S: Odd Fixes
 F: include/ui/qemu-spice.h
 F: include/ui/spice-display.h
 F: ui/spice-*.c
-- 
2.31.1

[PULL 0/7] Maintainers 20210811 patches

[Gerd Hoffmann]

The following changes since commit 703e8cd6189cf699c8d5c094bc68b5f3afa6ad71:

  Update version for v6.1.0-rc3 release (2021-08-10 19:08:09 +0100)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/maintainers-20210811-pull-request

for you to fetch changes up to a4de5e8a0667e3ee43ca9953ec7fd11ff19f2c92:

  MAINTAINERS: update virtio-gpu entry. (2021-08-11 08:39:16 +0200)


MAINTAINERS: update kraxel's entries.



Gerd Hoffmann (7):
  MAINTAINERS: update edk2 entry.
  MAINTAINERS: update sockets entry.
  MAINTAINERS: update audio entry.
  MAINTAINERS: update spice entry.
  MAINTAINERS: update usb entries.
  MAINTAINERS: update virtio-input entry.
  MAINTAINERS: update virtio-gpu entry.

 MAINTAINERS | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

-- 
2.31.1

Re: [PATCH v3 09/10] virtiofsd: Optionally fill lo_inode.fhandle

[Hanna Reitz]

On 10.08.21 17:57, Vivek Goyal wrote:

On Tue, Aug 10, 2021 at 05:26:15PM +0200, Hanna Reitz wrote:

On 10.08.21 17:23, Vivek Goyal wrote:

On Tue, Aug 10, 2021 at 10:32:55AM +0200, Hanna Reitz wrote:

On 09.08.21 20:41, Vivek Goyal wrote:

On Fri, Jul 30, 2021 at 05:01:33PM +0200, Max Reitz wrote:

When the inode_file_handles option is set, try to generate a file handle
for new inodes instead of opening an O_PATH FD.

Being able to open these again will require CAP_DAC_READ_SEARCH, so the
description text tells the user they will also need to specify
-o modcaps=+dac_read_search.

Generating a file handle returns the mount ID it is valid for.  Opening
it will require an FD instead.  We have mount_fds to map an ID to an FD.
get_file_handle() fills the hash map by opening the file we have
generated a handle for.  To verify that the resulting FD indeed
represents the handle's mount ID, we use statx().  Therefore, using file
handles requires statx() support.

So opening the file and storing that fd in mount_fds table might be
a potential problem with inotify work Ioannis is doing.

So say a file foo.txt was opened O_RDONLY and fd stored in mount_fs. Now
say user unlinks foo.txt. If notifications are enabled, final notification
will not be generated till this mount_fds fd is closed.

Now question is when will this fd be closed? If it closed at some
later point and then notification is generated, that will break
notificaitons.

Currently, it is never closed.


In fact even O_PATH fd is delaying notifications due to same reason.
But its not too bad as we close O_PATH fd pretty quickly after
unlinking. And we were hoping that file handle support will get rid
of this problem because we will not keep O_PATH fd open.

But, IIUC, mount_fds stuff will make it even worse. I did not see
the code which removes this fd from mount_fds. So I am not sure what's
the life time of this fd.

The lifetime is forever.  If we wanted to remove it at some point, we’d need
to track how many file handles we have open for the given mount fd and then
remove it from the table once the count reaches 0, so it would still be
delayed.

I think in practice the first thing that is looked up from some mount will
probably be the root directory, which cannot be deleted before everything
else on the mount is gone, so that would work.  We track how many handles
are there, if the whole mount were to be deleted, I hope all lo_inodes are
evicted, the count goes to 0, and we can drop the mount fd.

Keeping a reference count on mount_fd object make sense. So we probably
maintain this hash table and lookup using mount_id (as you are already
doing). All subsequent inodes from same filesystem will use same
object. Once all inodes have been flushed out, then mount_fd object
should go away as well (allowing for unmount on host).


I think we can make the assumption that the mount fd is the root directory
certain by, well, looking into mountinfo...  That would result in us always
opening the root node of the filesystem, so that first the whole filesystem
needs to disappear before it can be deleted (and our mount fd closed) –
which should work, I guess?

This seems more reasonable. And I think that's what man page seems to
suggest.

 The  mount_id  argument  returns an identifier for the filesystem mount
 that corresponds to pathname.  This corresponds to the first  field  in
 one  of  the  records in /proc/self/mountinfo.  Opening the pathname in
 the fifth field of that record yields a file descriptor for  the  mount
 point;  that  file  descriptor  can  be  used  in  a subsequent call to
 open_by_handle_at().

Fifth field seems to be the mount point. man proc says.

(5)  mount  point:  the  pathname of the mount point relative to
 the process's root directory.

So opening mount point and saving as mount_fd (if it is not already
in hash table) and then take a per inode reference count on mount_fd
object looks like will solve the life time issue of mount_fd as
well as the issue of temporary failures arising because we can't
open a device special file.

Well, we’ve had this discussion before, and it’s possible that a filesystem
has a device file as its mount point.

Yes. I think you did modified fuse to do some special trickery. Not sure
where should that be fixed.


I used fuse, but I’m sure a non-fuse filesystem can do the same.  (I 
mean, fuse effectively is a non-fuse filesystem, too.)


I don’t think it needs to be fixed, it just means we need to continue to 
stat the mount point to verify it’s a regular file or directory.



If filesystem is faking, then it can fake a device node as regular
file and fool us into opening it as well?


Well, of course opening any file can have side effects, on any filesystem.


But given the inotify complications, there’s really a good reason we should
use mountinfo.


It’s a bit tricky because our sandboxing prevents easy access to mountinf

Re: [PATCH v2 1/1] virtio: failover: define the default device to use in case of error

[Laurent Vivier]

On 11/08/2021 06:18, Jason Wang wrote:
> On Tue, Aug 10, 2021 at 1:14 AM Laurent Vivier  wrote:
>>
>> If the guest driver doesn't support the STANDBY feature, by default
>> we keep the virtio-net device and don't hotplug the VFIO device,
>> but in some cases, user can prefer to use the VFIO device rather
>> than the virtio-net one. We can't unplug the virtio-net device
>> (because on migration it is expected on the destination side)
>> but we can force the guest driver to be disabled. Then, we can
>> hotplug the VFIO device that will be unplugged before the migration
>> like in the normal failover migration but without the failover device.
>>
>> This patch adds a new property to virtio-net device: "failover-default".
>>
>> By default, "failover-default" is set to true and thus the default NIC
>> to use if the failover cannot be enabled is the virtio-net device
>> (this is what is done until now with the virtio-net failover).
>>
>> If "failover-default" is set to false, in case of error, the virtio-net
>> device is not the default anymore and the failover primary device
>> is used instead.
>>
>> If the STANDBY feature is supported by guest and host, the virtio-net
>> failover acts as usual.
>>
>> Signed-off-by: Laurent Vivier 
>> ---
>>  include/hw/virtio/virtio-net.h |  1 +
>>  hw/net/virtio-net.c| 49 +-
>>  2 files changed, 44 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
>> index 824a69c23f06..ab77930a327e 100644
>> --- a/include/hw/virtio/virtio-net.h
>> +++ b/include/hw/virtio/virtio-net.h
>> @@ -208,6 +208,7 @@ struct VirtIONet {
>>  /* primary failover device is hidden*/
>>  bool failover_primary_hidden;
>>  bool failover;
>> +bool failover_default;
>>  DeviceListener primary_listener;
>>  Notifier migration_state;
>>  VirtioNetRssData rss_data;
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 16d20cdee52a..972c03232a96 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -935,12 +935,23 @@ static void virtio_net_set_features(VirtIODevice 
>> *vdev, uint64_t features)
>>  memset(n->vlans, 0xff, MAX_VLAN >> 3);
>>  }
>>
>> -if (virtio_has_feature(features, VIRTIO_NET_F_STANDBY)) {
>> -qapi_event_send_failover_negotiated(n->netclient_name);
>> -qatomic_set(&n->failover_primary_hidden, false);
>> -failover_add_primary(n, &err);
>> -if (err) {
>> -warn_report_err(err);
>> +/*
>> + * if the virtio-net driver has the STANDBY feature, we can plug the 
>> primary
>> + * if not but is not the default failover device,
>> + * we need to plug the primary alone and the virtio-net driver will
>> + * be disabled in the validate_features() function but 
>> validate_features()
>> + * is only available with virtio 1.0 spec
>> + */
>> +if (n->failover) {
>> +if (virtio_has_feature(features, VIRTIO_NET_F_STANDBY) ||
>> +   (virtio_has_feature(features, VIRTIO_F_VERSION_1) &&
> 
> I think STANDY implies VERSION_1.
> 
> And if we do this, it means it doesn't work for legacy drivers.
> 
> Not sure if it's an issue.

Yes, you're right. In case of a kernel driver that doesn't support STANDBY and 
that is not
version 1, the virtio-net device is disabled (and the VFIO device is not 
plugged).

Thanks,
Laurent

Re: [PATCH v2 0/1] virtio: failover: allow to keep the VFIO device rather than the virtio-net one

[Laurent Vivier]

On 11/08/2021 06:17, Jason Wang wrote:
> On Tue, Aug 10, 2021 at 1:13 AM Laurent Vivier  wrote:
>>
>> v2: use validate_features() to disable the guest driver rather
>> than setting vring.num to 0.
>>
>> With failover, when the guest virtio-net driver doesn't support the
>> STANDBY feature, the primary device is not plugged and only the virtio-net
>> device is kept. Doing like that we can migrate the machine and
>> keep the network connection.
>>
>> But in some cases, when performance is more important than availability
>> we would prefer to keep the VFIO device rather than the virtio-net one,
>> even if it means to lose the network connection during the migration of
>> the machine.
> 
> I think we still need to seek a way to recover the network after migration.

If the network device is configured to have an IP address at boot, the IP will 
be restored
when the card is hotplugged. The only difference with virtio-net is _during_ the
migration. Witht virtio-net we keep the networking while the VM is migrated, 
without it we
lose the networking when the VFIO card is unplugged and restored when it is 
plugged back.

Thanks,
Laurent

> 
> Thanks
> 
>>
>> To do that we can't simply unplug the virtio-net device and plug the
>> VFIO one because for the migration the initial state must be kept
>> (virtio-net plugged, VFIO unplugged) but we can try to disable the
>> virtio-net driver and plug the VFIO card, so the initial state is
>> correct (the virtio-net card is plugged, but disabled in guest, and
>> the VFIO card is unplugged before migration).
>>
>> This change doesn't impact the case when guest and host support
>> the STANDBY feature.
>>
>> I've introduced the "failover-default" property to virtio-net device
>> to set which device to keep (failover-default=true keeps the virtio-net
>> device, =off the other one).
>>
>> For example, with a guest driver that doesn't support STANDBY:
>>
>>   ...
>>   -device virtio-net-pci,id=virtio0,failover=on,failover-default=on \
>>   -device vfio-pci,host=$PCI,id=hostdev0,failover_pair_id=virtio0 \
>>   ...
>>
>>   [root@localhost ~]# ip a
>>   1: lo:  mtu 65536 qdisc noqueue state UNKNOWN qlen 1
>>   link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>>   inet 127.0.0.1/8 scope host lo
>>  valid_lft forever preferred_lft forever
>>   inet6 ::1/128 scope host
>>  valid_lft forever preferred_lft forever
>>   2: eth0:  mtu 1500 qdisc pfifo_fast state 
>> UP q0
>>   link/ether 26:28:c5:7f:14:24 brd ff:ff:ff:ff:ff:ff
>>   inet 192.168.20.2/24 brd 192.168.20.255 scope global eth0
>>  valid_lft forever preferred_lft forever
>>   inet6 fe80::2428:c5ff:fe7f:1424/64 scope link
>>  valid_lft forever preferred_lft forever
>>   # ethtool -i eth0
>>   driver: virtio_net
>>   version: 1.0.0
>>   firmware-version:
>>   expansion-rom-version:
>>   bus-info: :04:00.0
>>   supports-statistics: no
>>   supports-test: no
>>   supports-eeprom-access: no
>>   supports-register-dump: no
>>   supports-priv-flags: no
>>
>>   ...
>>   -device virtio-net-pci,id=virtio0,failover=on,failover-default=off \
>>   -device vfio-pci,host=$PCI,id=hostdev0,failover_pair_id=virtio0 \
>>   ...
>>
>>   [root@localhost ~]# ip a
>>   1: lo:  mtu 65536 qdisc noqueue state UNKNOWN qlen 1
>>   link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>>   inet 127.0.0.1/8 scope host lo
>>  valid_lft forever preferred_lft forever
>>   inet6 ::1/128 scope host
>>  valid_lft forever preferred_lft forever
>>   2: enp2s0:  mtu 1500 qdisc mq state UP 
>> qlen 100
>>   link/ether 26:28:c5:7f:14:24 brd ff:ff:ff:ff:ff:ff
>>   inet 192.168.20.2/24 brd 192.168.20.255 scope global enp2s0
>>  valid_lft forever preferred_lft forever
>>   inet6 fe80::2428:c5ff:fe7f:1424/64 scope link
>>  valid_lft forever preferred_lft forever
>>   [root@localhost ~]# ethtool -i enp2s0
>>   driver: i40evf
>>   version: 1.6.27-k
>>   firmware-version: N/A
>>   expansion-rom-version:
>>   bus-info: :02:00.0
>>   supports-statistics: yes
>>   supports-test: no
>>   supports-eeprom-access: no
>>   supports-register-dump: no
>>   supports-priv-flags: no
>>
>> With guest driver that supports STANDBY, we would always have:
>>
>>   [root@localhost ~]# ip a
>>   1: lo:  mtu 65536 qdisc noqueue state UNKNOWN group 
>> defau0
>>   link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>>   inet 127.0.0.1/8 scope host lo
>>  valid_lft forever preferred_lft forever
>>   inet6 ::1/128 scope host
>>  valid_lft forever preferred_lft forever
>>   2: enp4s0:  mtu 1500 qdisc noqueue state 
>> UP gr0
>>   link/ether 26:28:c5:7f:14:24 brd ff:ff:ff:ff:ff:ff
>>   inet 192.168.20.2/24 brd 192.168.20.255 scope global noprefixroute 
>> enp4s0
>>  valid_lft forever preferred_lft forever
>>   inet6 fe80::2428:c5ff:fe7f:1424/64 scope link
>>  valid_lft forever preferred_lft forever
>>   3: enp4s0nsby:  m

Re: [PATCH 3/7] MAINTAINERS: update audio entry.

[clamky]

Gerd Hoffmann  writes:

> On Tue, Aug 10, 2021 at 03:17:43PM +0300, cla...@hotmail.com wrote:
>> Gerd Hoffmann  writes:
>> 
>> Hell Gerd.
>> 
>> > New maintainer wanted. Downgrade status to "Odd Fixes" for now.
>> 
>> I can try to retake it.
>
> qemu development happens in public on the qemu-devel mailing list,
> not behind closed doors.  Same goes for maintenance, updates to the
> MAINTAINERS file and related discussions are happening on the mailing
> list too.
>
> So, of you want seriously step up you should start by saying so in
> public, on the mailing list, not to me privately.
>
> take care,
>   Gerd

Re: [PATCH 1/2] gitlab: exclude sparc-softmmu and riscv32-softmmu from cross builds

[Philippe Mathieu-Daudé]

+Mark/Artyom

On 8/10/21 4:06 PM, Daniel P. Berrangé wrote:
> We need to cut down compile time by excluding more targets. Both these
> targets still have their 64-bit variant enabled, so the loss of coverage
> is mitigated to some degree.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  .gitlab-ci.d/crossbuild-template.yml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/.gitlab-ci.d/crossbuild-template.yml 
> b/.gitlab-ci.d/crossbuild-template.yml
> index 7d3ad00a1e..cfb576b54c 100644
> --- a/.gitlab-ci.d/crossbuild-template.yml
> +++ b/.gitlab-ci.d/crossbuild-template.yml
> @@ -9,7 +9,8 @@
>../configure --enable-werror --disable-docs $QEMU_CONFIGURE_OPTS
>  --disable-user --target-list-exclude="arm-softmmu cris-softmmu
>i386-softmmu microblaze-softmmu mips-softmmu mipsel-softmmu
> -  mips64-softmmu ppc-softmmu sh4-softmmu xtensa-softmmu"
> +  mips64-softmmu ppc-softmmu riscv32-softmmu sh4-softmmu
> +  sparc-softmmu xtensa-softmmu"

I disagree for sparc, as there are some differences (sparc64 is not
a superset of it). However I'm not against this patch.

>  - make -j$(expr $(nproc) + 1) all check-build $MAKE_CHECK_ARGS
>  - if grep -q "EXESUF=.exe" config-host.mak;
>then make installer;
> 

Re: [PATCH 2/2] gitlab: skip many more targets in windows cross builds

[Philippe Mathieu-Daudé]

On 8/10/21 4:06 PM, Daniel P. Berrangé wrote:
> The windows cross builds still take way too long in gitlab CI, so need
> more targets to be skipped. We don't want to hurt coverage of other
> cross builds more though, so we let jobs fine tune with a new env
> variale $CROSS_SKIP_TARGETS.
> 
> We take the set of targets that are considered relatively niche or
> very old architectures, and skip approx half of them in win32 builds
> and the other half of them in win64.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  .gitlab-ci.d/crossbuild-template.yml | 2 +-
>  .gitlab-ci.d/crossbuilds.yml | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/.gitlab-ci.d/crossbuild-template.yml 
> b/.gitlab-ci.d/crossbuild-template.yml
> index cfb576b54c..10d22dcf6c 100644
> --- a/.gitlab-ci.d/crossbuild-template.yml
> +++ b/.gitlab-ci.d/crossbuild-template.yml
> @@ -10,7 +10,7 @@
>  --disable-user --target-list-exclude="arm-softmmu cris-softmmu
>i386-softmmu microblaze-softmmu mips-softmmu mipsel-softmmu
>mips64-softmmu ppc-softmmu riscv32-softmmu sh4-softmmu
> -  sparc-softmmu xtensa-softmmu"
> +  sparc-softmmu xtensa-softmmu $CROSS_SKIP_TARGETS"
>  - make -j$(expr $(nproc) + 1) all check-build $MAKE_CHECK_ARGS
>  - if grep -q "EXESUF=.exe" config-host.mak;
>then make installer;
> diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
> index 4ff3aa3cfc..a5f9dbcbeb 100644
> --- a/.gitlab-ci.d/crossbuilds.yml
> +++ b/.gitlab-ci.d/crossbuilds.yml
> @@ -160,6 +160,7 @@ cross-win32-system:
>  job: win32-fedora-cross-container
>variables:
>  IMAGE: fedora-win32-cross
> +CROSS_SKIP_TARGETS: or1k-softmmu rx-softmmu sh4eb-softmmu 
> sparc64-softmmu tricore-softmmu xtensaeb-softmmu
>artifacts:
>  paths:
>- build/qemu-setup*.exe
> @@ -170,6 +171,7 @@ cross-win64-system:
>  job: win64-fedora-cross-container
>variables:
>  IMAGE: fedora-win64-cross
> +CROSS_SKIP_TARGETS: alpha-softmmu avr-softmmu hppa-softmmu m68k-softmmu 
> microblazeel-softmmu mips64el-softmmu nios2-softmmu

It looks you inverted the lists. We expect more Win64 users, and these
are the targets of interest. I'd keep here (skipping in Win32):

alpha-softmmu avr-softmmu hppa-softmmu m68k-softmmu microblazeel-softmmu
mips64el-softmmu sparc64-softmmu

And skip (keep them in win32):

or1k-softmmu rx-softmmu sh4eb-softmmu nios2-softmmu tricore-softmmu
xtensaeb-softmmu

>artifacts:
>  paths:
>- build/qemu-setup*.exe
> 

Re: [PATCH v12] qapi: introduce 'query-x86-cpuid' QMP command.

[Thomas Huth]

On 10/08/2021 20.56, Eduardo Habkost wrote:

On Sat, Aug 07, 2021 at 04:22:42PM +0200, Markus Armbruster wrote:

Is this intended to be a stable interface?  Interfaces intended just for
debugging usually aren't.


I don't think we need to make it a stable interface, but I won't
mind if we declare it stable.


If we don't feel 100% certain yet, it's maybe better to introduce this with 
a "x-" prefix first, isn't it? I.e. "x-query-x86-cpuid" ... then it's clear 
that this is only experimental/debugging/not-stable yet. Just my 0.02 €.


 Thomas

Re: [PATCH] build: remove redundant 'check-build' make target

[Thomas Huth]

On 10/08/2021 18.44, Daniel P. Berrangé wrote:

The 'check-build' make target was added as a way to build all the unit
test binaries, since the standard 'all' target would not trigger this.

Since the switch to meson, however, 'all' will now include the 'test'
binaries. As a result, 'check-build' is a no-op:

$ make all check-build
..snip lots of output...
make: Nothing to be done for 'check-build'.


I think it would be better to restore the previous behaviour, so that "all" 
does not build the test files by default. Most normal users don't need the 
tests, so compiling them by default wastes precious CPU cycles.


See also:


https://lore.kernel.org/qemu-devel/8d5714d1-d92b-60fc-531f-73eb05852...@gmail.com/


https://lore.kernel.org/qemu-devel/472c9809-8987-3c2c-c2b5-b99d637ac...@redhat.com/

 Thomas

Re: [PATCH 0/2] gitlab: avoid timeouts in windows cross builds

[Thomas Huth]

On 10/08/2021 16.06, Daniel P. Berrangé wrote:

The win64 cross build is quite often hitting the job timeout, despite
having an elevated timeout of 80 minutes. A typical time is 75-78
minutes in my tests, which leaves little headroom.

I'm not inclined to increase the timeout beyond 80 minutes as this is
already unreasonably long.

Interestingly the win32 job doesn't timeout in the same way. In local
tests I've found the mingw32 gcc is as much as 50% faster than
mingw64 gcc in building QEMU. This explains at least why we only see
win64 gitlab builds timeout normally.


I've had similar experiences in the past, and once even already sent a patch:


https://patchwork.ozlabs.org/project/qemu-devel/patch/20210414081907.871437-4-th...@redhat.com/

But your apprach is looking way nicer, indeed!

Queued to my testing-next branch now:

 https://gitlab.com/thuth/qemu/-/commits/testing-next/

Thanks,
 Thomas

Re: [PATCH v3] net/macos: implement vmnet-based netdev

[Jason Wang]

在 2021/7/8 下午1:44, Akihiko Odaki 写道:

From: Phillip Tennen 

This patch implements a new netdev device, reachable via -netdev
vmnet-macos, that’s backed by macOS’s vmnet framework.

The vmnet framework provides native bridging support, and its usage in
this patch is intended as a replacement for attempts to use a tap device
via the tuntaposx kernel extension. Notably, the tap/tuntaposx approach
never would have worked in the first place, as QEMU interacts with the
tap device via poll(), and macOS does not support polling device files.

vmnet requires either a special entitlement, granted via a provisioning
profile, or root access. Otherwise attempts to create the virtual
interface will fail with a “generic error” status code. QEMU may not
currently be signed with an entitlement granted in a provisioning
profile, as this would necessitate pre-signed binary build distribution,
rather than source-code distribution.



This is something that needs to be addressed.

A question, does mac has something similar to SCM_RIGHTS that allows the 
handle to be created by privileged process and passed to unprivileged 
process? like tap,fd=XYZ.




As such, using this netdev
currently requires that qemu be run with root access. I’ve opened a
feedback report with Apple to allow the use of the relevant entitlement
with this use case:
https://openradar.appspot.com/radar?id=5007417364447232

vmnet offers three operating modes, all of which are supported by this
patch via the “mode=host|shared|bridge” option:

* "Host" mode: Allows the vmnet interface to communicate with other
* vmnet
interfaces that are in host mode and also with the native host.
* "Shared" mode: Allows traffic originating from the vmnet interface to
reach the Internet through a NAT. The vmnet interface can also
communicate with the native host.
* "Bridged" mode: Bridges the vmnet interface with a physical network
interface.

Each of these modes also provide some extra configuration that’s
supported by this patch:

* "Bridged" mode: The user may specify the physical interface to bridge
with. Defaults to en0.
* "Host" mode / "Shared" mode: The user may specify the DHCP range and
subnet. Allocated by vmnet if not provided.

vmnet also offers some extra configuration options that are not
supported by this patch:

* Enable isolation from other VMs using vmnet
* Port forwarding rules
* Enabling TCP segmentation offload
* Only applicable in "shared" mode: specifying the NAT IPv6 prefix
* Only available in "host" mode: specifying the IP address for the VM
within an isolated network

Note that this patch requires macOS 10.15 as a minimum, as this is when
bridging support was implemented in vmnet.framework.

Rebased to commit 9aef0954195cc592e86846dbbe7f3c2c5603690a by Akihiko
Odaki.

Signed-off-by: Phillip Tennen 
Signed-off-by: Akihiko Odaki 
Message-Id: <20210315103209.20870-1-akihiko.od...@gmail.com>
---
  meson.build   |   3 +
  net/clients.h |   5 +
  net/meson.build   |   1 +
  net/net.c |   3 +
  net/vmnet-macos.c | 447 ++
  qapi/net.json | 120 -
  qemu-options.hx   |   9 +



I think we'd better list you as the maintainer for this backend.



  7 files changed, 586 insertions(+), 2 deletions(-)
  create mode 100644 net/vmnet-macos.c

diff --git a/meson.build b/meson.build
index b9a9b2120fe..0d2ceaa880e 100644
--- a/meson.build
+++ b/meson.build
@@ -178,6 +178,7 @@ socket = []
  version_res = []
  coref = []
  iokit = []
+vmnet = not_found
  emulator_link_args = []
  nvmm =not_found
  hvf = not_found
@@ -192,6 +193,7 @@ if targetos == 'windows'
  elif targetos == 'darwin'
coref = dependency('appleframeworks', modules: 'CoreFoundation')
iokit = dependency('appleframeworks', modules: 'IOKit', required: false)
+  vmnet = dependency('appleframeworks', modules: 'vmnet')
  elif targetos == 'sunos'
socket = [cc.find_library('socket'),
  cc.find_library('nsl'),
@@ -1259,6 +1261,7 @@ config_host_data.set('CONFIG_FUSE', fuse.found())
  config_host_data.set('CONFIG_FUSE_LSEEK', fuse_lseek.found())
  config_host_data.set('CONFIG_X11', x11.found())
  config_host_data.set('CONFIG_CFI', get_option('cfi'))
+config_host_data.set('CONFIG_VMNET', vmnet.found())
  config_host_data.set('QEMU_VERSION', '"@0@"'.format(meson.project_version()))
  config_host_data.set('QEMU_VERSION_MAJOR', 
meson.project_version().split('.')[0])
  config_host_data.set('QEMU_VERSION_MINOR', 
meson.project_version().split('.')[1])
diff --git a/net/clients.h b/net/clients.h
index 92f9b59aedc..2c2af67f82a 100644
--- a/net/clients.h
+++ b/net/clients.h
@@ -63,4 +63,9 @@ int net_init_vhost_user(const Netdev *netdev, const char 
*name,
  
  int net_init_vhost_vdpa(const Netdev *netdev, const char *name,

  NetClientState *peer, Error **errp);
+
+#ifdef CONFIG_VMNET
+int net_init_vmnet_macos(const Netdev *netdev, const char *name,
+NetClientState *peer, Error *

Re: [PATCH v2 1/1] virtio: failover: define the default device to use in case of error

[Jason Wang]

On Tue, Aug 10, 2021 at 1:14 AM Laurent Vivier  wrote:
>
> If the guest driver doesn't support the STANDBY feature, by default
> we keep the virtio-net device and don't hotplug the VFIO device,
> but in some cases, user can prefer to use the VFIO device rather
> than the virtio-net one. We can't unplug the virtio-net device
> (because on migration it is expected on the destination side)
> but we can force the guest driver to be disabled. Then, we can
> hotplug the VFIO device that will be unplugged before the migration
> like in the normal failover migration but without the failover device.
>
> This patch adds a new property to virtio-net device: "failover-default".
>
> By default, "failover-default" is set to true and thus the default NIC
> to use if the failover cannot be enabled is the virtio-net device
> (this is what is done until now with the virtio-net failover).
>
> If "failover-default" is set to false, in case of error, the virtio-net
> device is not the default anymore and the failover primary device
> is used instead.
>
> If the STANDBY feature is supported by guest and host, the virtio-net
> failover acts as usual.
>
> Signed-off-by: Laurent Vivier 
> ---
>  include/hw/virtio/virtio-net.h |  1 +
>  hw/net/virtio-net.c| 49 +-
>  2 files changed, 44 insertions(+), 6 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> index 824a69c23f06..ab77930a327e 100644
> --- a/include/hw/virtio/virtio-net.h
> +++ b/include/hw/virtio/virtio-net.h
> @@ -208,6 +208,7 @@ struct VirtIONet {
>  /* primary failover device is hidden*/
>  bool failover_primary_hidden;
>  bool failover;
> +bool failover_default;
>  DeviceListener primary_listener;
>  Notifier migration_state;
>  VirtioNetRssData rss_data;
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 16d20cdee52a..972c03232a96 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -935,12 +935,23 @@ static void virtio_net_set_features(VirtIODevice *vdev, 
> uint64_t features)
>  memset(n->vlans, 0xff, MAX_VLAN >> 3);
>  }
>
> -if (virtio_has_feature(features, VIRTIO_NET_F_STANDBY)) {
> -qapi_event_send_failover_negotiated(n->netclient_name);
> -qatomic_set(&n->failover_primary_hidden, false);
> -failover_add_primary(n, &err);
> -if (err) {
> -warn_report_err(err);
> +/*
> + * if the virtio-net driver has the STANDBY feature, we can plug the 
> primary
> + * if not but is not the default failover device,
> + * we need to plug the primary alone and the virtio-net driver will
> + * be disabled in the validate_features() function but 
> validate_features()
> + * is only available with virtio 1.0 spec
> + */
> +if (n->failover) {
> +if (virtio_has_feature(features, VIRTIO_NET_F_STANDBY) ||
> +   (virtio_has_feature(features, VIRTIO_F_VERSION_1) &&

I think STANDY implies VERSION_1.

And if we do this, it means it doesn't work for legacy drivers.

Not sure if it's an issue.

Thanks

> +!n->failover_default)) {
> +qapi_event_send_failover_negotiated(n->netclient_name);
> +qatomic_set(&n->failover_primary_hidden, false);
> +failover_add_primary(n, &err);
> +if (err) {
> +warn_report_err(err);
> +}
>  }
>  }
>  }
> @@ -3625,9 +3636,34 @@ static Property virtio_net_properties[] = {
>  DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed, SPEED_UNKNOWN),
>  DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str),
>  DEFINE_PROP_BOOL("failover", VirtIONet, failover, false),
> +DEFINE_PROP_BOOL("failover-default", VirtIONet, failover_default, true),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>
> +/* validate_features() is only available with VIRTIO_F_VERSION_1 */
> +static int failover_validate_features(VirtIODevice *vdev)
> +{
> +VirtIONet *n = VIRTIO_NET(vdev);
> +
> +/*
> + * If the guest driver doesn't support the STANDBY feature, by default
> + * we keep the virtio-net device and don't hotplug the VFIO device,
> + * but in some cases, user can prefer to use the VFIO device rather
> + * than the virtio-net one. We can't unplug the virtio-net device
> + * (because on migration it is expected on the destination side)
> + * but we can force the guest driver to be disabled. In this case, We can
> + * hotplug the VFIO device that will be unplugged before the migration
> + * like in the normal failover migration but without the failover device.
> + */
> +if (n->failover && !n->failover_default &&
> +!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_STANDBY)) {
> +/* disable virtio-net */
> +return -ENODEV;
> +}
> +
> +return 0;
> +}
> +
>  static void virtio_net_class_init(ObjectClass *klass, void *data)
>  {
>  DeviceClass *dc = DEVICE_CL

Re: [PATCH v2 0/1] virtio: failover: allow to keep the VFIO device rather than the virtio-net one

[Jason Wang]

On Tue, Aug 10, 2021 at 1:13 AM Laurent Vivier  wrote:
>
> v2: use validate_features() to disable the guest driver rather
> than setting vring.num to 0.
>
> With failover, when the guest virtio-net driver doesn't support the
> STANDBY feature, the primary device is not plugged and only the virtio-net
> device is kept. Doing like that we can migrate the machine and
> keep the network connection.
>
> But in some cases, when performance is more important than availability
> we would prefer to keep the VFIO device rather than the virtio-net one,
> even if it means to lose the network connection during the migration of
> the machine.

I think we still need to seek a way to recover the network after migration.

Thanks

>
> To do that we can't simply unplug the virtio-net device and plug the
> VFIO one because for the migration the initial state must be kept
> (virtio-net plugged, VFIO unplugged) but we can try to disable the
> virtio-net driver and plug the VFIO card, so the initial state is
> correct (the virtio-net card is plugged, but disabled in guest, and
> the VFIO card is unplugged before migration).
>
> This change doesn't impact the case when guest and host support
> the STANDBY feature.
>
> I've introduced the "failover-default" property to virtio-net device
> to set which device to keep (failover-default=true keeps the virtio-net
> device, =off the other one).
>
> For example, with a guest driver that doesn't support STANDBY:
>
>   ...
>   -device virtio-net-pci,id=virtio0,failover=on,failover-default=on \
>   -device vfio-pci,host=$PCI,id=hostdev0,failover_pair_id=virtio0 \
>   ...
>
>   [root@localhost ~]# ip a
>   1: lo:  mtu 65536 qdisc noqueue state UNKNOWN qlen 1
>   link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>   inet 127.0.0.1/8 scope host lo
>  valid_lft forever preferred_lft forever
>   inet6 ::1/128 scope host
>  valid_lft forever preferred_lft forever
>   2: eth0:  mtu 1500 qdisc pfifo_fast state 
> UP q0
>   link/ether 26:28:c5:7f:14:24 brd ff:ff:ff:ff:ff:ff
>   inet 192.168.20.2/24 brd 192.168.20.255 scope global eth0
>  valid_lft forever preferred_lft forever
>   inet6 fe80::2428:c5ff:fe7f:1424/64 scope link
>  valid_lft forever preferred_lft forever
>   # ethtool -i eth0
>   driver: virtio_net
>   version: 1.0.0
>   firmware-version:
>   expansion-rom-version:
>   bus-info: :04:00.0
>   supports-statistics: no
>   supports-test: no
>   supports-eeprom-access: no
>   supports-register-dump: no
>   supports-priv-flags: no
>
>   ...
>   -device virtio-net-pci,id=virtio0,failover=on,failover-default=off \
>   -device vfio-pci,host=$PCI,id=hostdev0,failover_pair_id=virtio0 \
>   ...
>
>   [root@localhost ~]# ip a
>   1: lo:  mtu 65536 qdisc noqueue state UNKNOWN qlen 1
>   link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>   inet 127.0.0.1/8 scope host lo
>  valid_lft forever preferred_lft forever
>   inet6 ::1/128 scope host
>  valid_lft forever preferred_lft forever
>   2: enp2s0:  mtu 1500 qdisc mq state UP 
> qlen 100
>   link/ether 26:28:c5:7f:14:24 brd ff:ff:ff:ff:ff:ff
>   inet 192.168.20.2/24 brd 192.168.20.255 scope global enp2s0
>  valid_lft forever preferred_lft forever
>   inet6 fe80::2428:c5ff:fe7f:1424/64 scope link
>  valid_lft forever preferred_lft forever
>   [root@localhost ~]# ethtool -i enp2s0
>   driver: i40evf
>   version: 1.6.27-k
>   firmware-version: N/A
>   expansion-rom-version:
>   bus-info: :02:00.0
>   supports-statistics: yes
>   supports-test: no
>   supports-eeprom-access: no
>   supports-register-dump: no
>   supports-priv-flags: no
>
> With guest driver that supports STANDBY, we would always have:
>
>   [root@localhost ~]# ip a
>   1: lo:  mtu 65536 qdisc noqueue state UNKNOWN group 
> defau0
>   link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>   inet 127.0.0.1/8 scope host lo
>  valid_lft forever preferred_lft forever
>   inet6 ::1/128 scope host
>  valid_lft forever preferred_lft forever
>   2: enp4s0:  mtu 1500 qdisc noqueue state 
> UP gr0
>   link/ether 26:28:c5:7f:14:24 brd ff:ff:ff:ff:ff:ff
>   inet 192.168.20.2/24 brd 192.168.20.255 scope global noprefixroute 
> enp4s0
>  valid_lft forever preferred_lft forever
>   inet6 fe80::2428:c5ff:fe7f:1424/64 scope link
>  valid_lft forever preferred_lft forever
>   3: enp4s0nsby:  mtu 1500 qdisc fq_codel 
> master0
>   link/ether 26:28:c5:7f:14:24 brd ff:ff:ff:ff:ff:ff
>   4: enp2s0:  mtu 1500 qdisc mq master 
> enp4s0 st0
>   link/ether 26:28:c5:7f:14:24 brd ff:ff:ff:ff:ff:ff
>   [root@localhost ~]# ethtool -i enp4s0
>   driver: net_failover
>   version: 0.1
>   firmware-version:
>   expansion-rom-version:
>   bus-info:
>   supports-statistics: no
>   supports-test: no
>   supports-eeprom-access: no
>   supports-register-dump: no
>   supports-priv-flags: no
>   [root@localhost ~]# ethtool -i enp4s0ns

Re: [PATCH] hw/net: Discard overly fragmented packets

[Jason Wang]

在 2021/8/4 上午9:43, Jason Wang 写道:


在 2021/8/3 下午5:51, Philippe Mathieu-Daudé 写道:

On 8/3/21 11:33 AM, Thomas Huth wrote:

On 05/07/2021 10.40, Philippe Mathieu-Daudé wrote:

Our infrastructure can handle fragmented packets up to
NET_MAX_FRAG_SG_LIST (64) pieces. This hard limit has
been proven enough in production for years. If it is
reached, it is likely an evil crafted packet. Discard it.

Include the qtest reproducer provided by Alexander Bulekov:

    $ make check-qtest-i386
    ...
    Running test qtest-i386/fuzz-vmxnet3-test
    qemu-system-i386: net/eth.c:334: void
eth_setup_ip4_fragmentation(const void *, size_t, void *, size_t,
size_t, size_t, _Bool):
    Assertion `frag_offset % IP_FRAG_UNIT_SIZE == 0' failed.

Cc: qemu-sta...@nongnu.org
Reported-by: OSS-Fuzz (Issue 35799)
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/460
Signed-off-by: Philippe Mathieu-Daudé 
---
   hw/net/net_tx_pkt.c |   8 ++
   tests/qtest/fuzz-vmxnet3-test.c | 195 


   MAINTAINERS |   1 +
   tests/qtest/meson.build |   1 +
   4 files changed, 205 insertions(+)
   create mode 100644 tests/qtest/fuzz-vmxnet3-test.c

Reviewed-by: Thomas Huth 

Jason, I think this would even still qualify for QEMU v6.1 ?

Yes, easy one for 6.1.



Yes, this will be included for rc3.

Thanks



For some reasons it misses rc3.

I will include it for 6.2.

Sorry.

Re: [PATCH 09/10] hw/misc: Add Infineon DPS310 sensor model

[Joel Stanley]

On Tue, 10 Aug 2021 at 23:37, Corey Minyard  wrote:
>
> On Mon, Aug 09, 2021 at 03:15:55PM +0200, Cédric Le Goater wrote:
> > From: Joel Stanley 
> >
> > This contains some hardcoded register values that were obtained from the
> > hardware after reading the temperature.
> >
> > It does enough to test the Linux kernel driver. The FIFO mode, IRQs and
> > operation modes other than the default as used by Linux are not modelled.
> >
> > Signed-off-by: Joel Stanley 
> > [ clg: Fix sequential reading ]
> > Message-Id: <20210616073358.750472-2-j...@jms.id.au>
> > Signed-off-by: Cédric Le Goater 
> > Message-Id: <20210629142336.750058-4-...@kaod.org>
> > Signed-off-by: Cédric Le Goater 
> > ---
> >  hw/misc/dps310.c| 227 
>
> Can this go into hw/sensor?

For sure. I wrote it four years ago, when obviously hw/sensor didn't exist.

>
> -corey
>
> >  hw/arm/Kconfig  |   1 +
> >  hw/misc/Kconfig |   4 +
> >  hw/misc/meson.build |   1 +
> >  4 files changed, 233 insertions(+)
> >  create mode 100644 hw/misc/dps310.c
> >
> > diff --git a/hw/misc/dps310.c b/hw/misc/dps310.c
> > new file mode 100644
> > index ..893521ab8516
> > --- /dev/null
> > +++ b/hw/misc/dps310.c
> > @@ -0,0 +1,227 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright 2017-2021 Joel Stanley , IBM Corporation
> > + *
> > + * Infineon DPS310 temperature and humidity sensor
> > + *
> > + * 
> > https://www.infineon.com/cms/en/product/sensor/pressure-sensors/pressure-sensors-for-iot/dps310/
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/log.h"
> > +#include "hw/hw.h"
> > +#include "hw/i2c/i2c.h"
> > +#include "qapi/error.h"
> > +#include "qapi/visitor.h"
> > +#include "migration/vmstate.h"
> > +
> > +#define NUM_REGISTERS   0x33
> > +
> > +typedef struct DPS310State {
> > +/*< private >*/
> > +I2CSlave i2c;
> > +
> > +/*< public >*/
> > +uint8_t regs[NUM_REGISTERS];
> > +
> > +uint8_t len;
> > +uint8_t pointer;
> > +
> > +} DPS310State;
> > +
> > +#define TYPE_DPS310 "dps310"
> > +#define DPS310(obj) OBJECT_CHECK(DPS310State, (obj), TYPE_DPS310)
> > +
> > +#define DPS310_PRS_B2   0x00
> > +#define DPS310_PRS_B1   0x01
> > +#define DPS310_PRS_B0   0x02
> > +#define DPS310_TMP_B2   0x03
> > +#define DPS310_TMP_B1   0x04
> > +#define DPS310_TMP_B0   0x05
> > +#define DPS310_PRS_CFG  0x06
> > +#define DPS310_TMP_CFG  0x07
> > +#define  DPS310_TMP_RATE_BITS   (0x70)
> > +#define DPS310_MEAS_CFG 0x08
> > +#define  DPS310_MEAS_CTRL_BITS  (0x07)
> > +#define   DPS310_PRESSURE_ENBIT(0)
> > +#define   DPS310_TEMP_ENBIT(1)
> > +#define   DPS310_BACKGROUND BIT(2)
> > +#define  DPS310_PRS_RDY BIT(4)
> > +#define  DPS310_TMP_RDY BIT(5)
> > +#define  DPS310_SENSOR_RDY  BIT(6)
> > +#define  DPS310_COEF_RDYBIT(7)
> > +#define DPS310_CFG_REG  0x09
> > +#define DPS310_RESET0x0c
> > +#define  DPS310_RESET_MAGIC (BIT(0) | BIT(3))
> > +#define DPS310_COEF_BASE0x10
> > +#define DPS310_COEF_LAST0x21
> > +#define DPS310_COEF_SRC 0x28
> > +
> > +static void dps310_reset(DeviceState *dev)
> > +{
> > +DPS310State *s = DPS310(dev);
> > +
> > +static const uint8_t regs_reset_state[] = {
> > +0xfe, 0x2f, 0xee, 0x02, 0x69, 0xa6, 0x00, 0x80, 0xc7, 0x00, 0x00, 
> > 0x00,
> > +0x00, 0x10, 0x00, 0x00, 0x0e, 0x1e, 0xdd, 0x13, 0xca, 0x5f, 0x21, 
> > 0x52,
> > +0xf9, 0xc6, 0x04, 0xd1, 0xdb, 0x47, 0x00, 0x5b, 0xfb, 0x3a, 0x00, 
> > 0x00,
> > +0x20, 0x49, 0x4e, 0xa5, 0x90, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
> > 0x00,
> > +0x60, 0x15, 0x02
> > +};
> > +
> > +QEMU_BUILD_BUG_ON(sizeof(regs_reset_state) != sizeof(s->regs));
> > +
> > +memcpy(s->regs, regs_reset_state, sizeof(s->regs));
> > +s->pointer = 0;
> > +
> > +/* TODO: assert these after some timeout ? */
> > +s->regs[DPS310_MEAS_CFG] = DPS310_COEF_RDY | DPS310_SENSOR_RDY
> > +| DPS310_TMP_RDY | DPS310_PRS_RDY;
> > +}
> > +
> > +static uint8_t dps310_read(DPS310State *s, uint8_t reg)
> > +{
> > +if (reg >= sizeof(s->regs)) {
> > +qemu_log_mask(LOG_GUEST_ERROR, "%s: register 0x%02x out of 
> > bounds\n",
> > +  __func__, s->pointer);
> > +return 0xFF;
> > +}
> > +
> > +switch (reg) {
> > +case DPS310_PRS_B2:
> > +case DPS310_PRS_B1:
> > +case DPS310_PRS_B0:
> > +case DPS310_TMP_B2:
> > +case DPS310_TMP_B1:
> > +case DPS310_TMP_B0:
> > +case DPS310_PRS_CFG:
> > +case DPS310_TMP_CFG:
> > +case DPS310_MEAS_CFG:
> > +case DPS310_CFG_REG:
> > +case DPS310_COEF_BASE...DPS310_COEF_LAST:
> > +case DPS310_COEF_SRC:
> > +case 0x32: /* Undocumented register to indicate workaround not 
> > required */
> > +return s->regs[reg];
> > +default:
> > +qemu_log_mask(LOG_

Re: [PATCH 10/19] target/ppc: PMU Event-Based exception support

[Richard Henderson]

On 8/9/21 3:10 AM, Daniel Henrique Barboza wrote:

diff --git a/target/ppc/pmu_book3s_helper.c b/target/ppc/pmu_book3s_helper.c
index 91bb82e699..43cc0eb722 100644
--- a/target/ppc/pmu_book3s_helper.c
+++ b/target/ppc/pmu_book3s_helper.c
@@ -10,12 +10,15 @@
   * See the COPYING file in the top-level directory.
   */
  
+#include "pmu_book3s_helper.h"

+
  #include "qemu/osdep.h"


Never put anything before osdep.h.


+static void cpu_ppc_pmu_timer_cb(void *opaque)
+{
+PowerPCCPU *cpu = opaque;
+CPUPPCState *env = &cpu->env;
+uint64_t mmcr0;
+
+mmcr0 = env->spr[SPR_POWER_MMCR0];
+if (env->spr[SPR_POWER_MMCR0] & MMCR0_EBE) {
+/* freeeze counters if needed */
+if (mmcr0 & MMCR0_FCECE) {
+mmcr0 &= ~MMCR0_FCECE;
+mmcr0 |= MMCR0_FC;
+}
+
+/* Clear PMAE and set PMAO */
+if (mmcr0 & MMCR0_PMAE) {
+mmcr0 &= ~MMCR0_PMAE;
+mmcr0 |= MMCR0_PMAO;
+}
+env->spr[SPR_POWER_MMCR0] = mmcr0;
+
+/* Fire the PMC hardware exception */
+ppc_set_irq(cpu, PPC_INTERRUPT_PMC, 1);


Does this even compile ppc64-linux-user?
This function is in hw/ppc/.


r~

Re: [PATCH 09/19] PPC64/TCG: Implement 'rfebb' instruction

[Richard Henderson]

On 8/10/21 9:32 AM, Daniel Henrique Barboza wrote:

I'm not sure what is the disastree path.


David meant decodetree.  See insn32.decode.


r~

Re: [PATCH 09/19] PPC64/TCG: Implement 'rfebb' instruction

[Richard Henderson]

On 8/9/21 3:10 AM, Daniel Henrique Barboza wrote:

+TCGv target = tcg_temp_new();
+TCGv bescr = tcg_temp_new();
+
+gen_load_spr(target, SPR_EBBRR);
+tcg_gen_mov_tl(cpu_nip, target);


You don't need a temporary for target here; just load into cpu_nip directly.


+ctx->base.is_jmp = DISAS_EXIT;


You ought to be able to use DISAS_CHAIN.


r~

Re: [PATCH 08/19] target/ppc/pmu_book3s_helper.c: do an actual cycles calculation

[Richard Henderson]

On 8/9/21 3:10 AM, Daniel Henrique Barboza wrote:

+/*
+ * Set arbitrarily based on clock-frequency values used in PNV
+ * and SPAPR code.
+ */
+#define PPC_CPU_FREQ 10
  
  static uint64_t get_cycles(uint64_t icount_delta)

  {
-/* Placeholder value */
-return icount_delta * 4;
+return muldiv64(icount_to_ns(icount_delta), PPC_CPU_FREQ,
+NANOSECONDS_PER_SECOND);


So, unless you're going to do something real, you might as well just return icount_to_ns, 
and skip all of the no-op scaling.



r~

Re: [PATCH 04/19] target/ppc: PMU Book3s basic insns count for pseries TCG

[Richard Henderson]

On 8/9/21 3:10 AM, Daniel Henrique Barboza wrote:

The PMCC (PMC Control) bit in the MMCR0 register controls whether the
counters PMC5 and PMC6 are being part of the performance monitor
facility in a specific time. If PMCC allows it, PMC5 and PMC6 will
always be used to measure instructions completed and cycles,
respectively.

This patch adds the barebones of the Book3s PMU logic by enabling
instruction counting, using the icount framework, using the performance
monitor counters 5 and 6. The overall logic goes as follows:

- a helper is added to control the PMU state on each MMCR0 write. This
allows for the PMU to start/stop as quick as possible;

- only PMC5 and PMC6 are being set. PMC6 (cycles) is default to 4*insns
(for cycles per instruction) for now;

- the intended usage is to freeze the counters by setting MMCR0_FC, do
any additional setting via MMCR1 (not implemented yet) and setting
initial counter values,  and enable the PMU by zeroing MMCR0_FC. Software
must freeze counters to read the results - on the fly reading of the PMCs
will return the starting value of each one.

Since there will be more PMU exclusive code to be added next, let's also
put the PMU logic in its own helper to keep all in the same place. The
code is also repetitive and not really extensible to add more PMCs, but
we'll handle this in the next patches.

Signed-off-by: Daniel Henrique Barboza 
---
  target/ppc/cpu.h   |  4 ++
  target/ppc/cpu_init.c  |  4 +-
  target/ppc/helper.h|  1 +
  target/ppc/meson.build |  1 +
  target/ppc/pmu_book3s_helper.c | 78 ++
  target/ppc/translate.c | 14 --
  6 files changed, 97 insertions(+), 5 deletions(-)
  create mode 100644 target/ppc/pmu_book3s_helper.c

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 4d96015f81..229abfe7ee 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1175,6 +1175,10 @@ struct CPUPPCState {
  uint32_t tm_vscr;
  uint64_t tm_dscr;
  uint64_t tm_tar;
+
+/* PMU registers icount state */
+uint64_t pmc5_base_icount;
+uint64_t pmc6_base_icount;
  };
  
  #define SET_FIT_PERIOD(a_, b_, c_, d_)  \

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 71062809c8..fce89ee994 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6822,7 +6822,7 @@ static void register_book3s_pmu_sup_sprs(CPUPPCState *env)
  spr_register_kvm(env, SPR_POWER_MMCR0, "MMCR0",
   SPR_NOACCESS, SPR_NOACCESS,
   &spr_read_pmu_generic, &spr_write_pmu_generic,
- KVM_REG_PPC_MMCR0, 0x);
+ KVM_REG_PPC_MMCR0, 0x8000);
  spr_register_kvm(env, SPR_POWER_MMCR1, "MMCR1",
   SPR_NOACCESS, SPR_NOACCESS,
   &spr_read_pmu_generic, &spr_write_pmu_generic,
@@ -6870,7 +6870,7 @@ static void register_book3s_pmu_user_sprs(CPUPPCState 
*env)
  spr_register(env, SPR_POWER_UMMCR0, "UMMCR0",
   &spr_read_pmu_ureg, &spr_write_pmu_ureg,
   &spr_read_ureg, &spr_write_ureg,
- 0x);
+ 0x8000);
  spr_register(env, SPR_POWER_UMMCR1, "UMMCR1",
   &spr_read_pmu_ureg, &spr_write_pmu_ureg,
   &spr_read_ureg, &spr_write_ureg,
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 4076aa281e..5122632784 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -20,6 +20,7 @@ DEF_HELPER_1(rfscv, void, env)
  DEF_HELPER_1(hrfid, void, env)
  DEF_HELPER_2(store_lpcr, void, env, tl)
  DEF_HELPER_2(store_pcr, void, env, tl)
+DEF_HELPER_2(store_mmcr0, void, env, tl)
  #endif
  DEF_HELPER_1(check_tlb_flush_local, void, env)
  DEF_HELPER_1(check_tlb_flush_global, void, env)
diff --git a/target/ppc/meson.build b/target/ppc/meson.build
index b85f295703..bf252ca3ac 100644
--- a/target/ppc/meson.build
+++ b/target/ppc/meson.build
@@ -14,6 +14,7 @@ ppc_ss.add(when: 'CONFIG_TCG', if_true: files(
'int_helper.c',
'mem_helper.c',
'misc_helper.c',
+  'pmu_book3s_helper.c',
'timebase_helper.c',
'translate.c',
  ))
diff --git a/target/ppc/pmu_book3s_helper.c b/target/ppc/pmu_book3s_helper.c
new file mode 100644
index 00..fe16fcfce0
--- /dev/null
+++ b/target/ppc/pmu_book3s_helper.c
@@ -0,0 +1,78 @@
+/*
+ * PowerPC Book3s PMU emulation helpers for QEMU TCG
+ *
+ *  Copyright IBM Corp. 2021
+ *
+ * Authors:
+ *  Daniel Henrique Barboza  
+ *
+ * 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 "cpu.h"
+#include "exec/exec-all.h"
+#include "exec/helper-proto.h"
+#include "qemu/error-report.h"
+#include "qemu/main-loop.h"
+
+static uint64_t get_insns(void)
+{
+return (uint64_t)icount_get_raw();
+}


So... this will just abort for user-only, which I suppose is fair since you won't be 
setting the o

Re: [PATCH] fuzz: avoid building twice, when running on gitlab

[Coiby Xu]

On Tue, Aug 10, 2021 at 07:01:45AM +0200, Philippe Mathieu-Daudé wrote:

+Coiby Xu & qemu-block@


Thanks for adding me to the Cc list.



On 8/9/21 9:36 PM, Peter Maydell wrote:

On Mon, 9 Aug 2021 at 20:30, Alexander Bulekov  wrote:


On 210809 1506, Alexander Bulekov wrote:

On 210809 1925, Peter Maydell wrote:

On Mon, 9 Aug 2021 at 12:18, Alexander Bulekov  wrote:


On oss-fuzz, we build twice, to put together a build that is portable to
the runner containers. On gitlab ci, this is wasteful and contributes to
timeouts on the build-oss-fuzz job. Avoid building twice on gitlab, at
the remote cost of potentially missing some cases that break oss-fuzz
builds.

Signed-off-by: Alexander Bulekov 
---

From a couple test runs it looks like this can shave off 15-20 minutes.

 scripts/oss-fuzz/build.sh | 24 +---
 1 file changed, 13 insertions(+), 11 deletions(-)


I tried a test run with this, but it still hit the 1 hour timeout:

https://gitlab.com/qemu-project/qemu/-/pipelines/350387482


It also timed out for me with a 120 minute timeout:
https://gitlab.com/a1xndr/qemu/-/jobs/1488160601

The log has almost exactly the same number of lines as yours, so I'm
guessing one of the qtests is timing out with --enable-sanitizers .




Building locally:
$ CC=clang-11 CXX=clang++-11 ../configure --enable-fuzzing \
--enable-debug --enable-sanitizers
$ make check-qtest-i386 check-unit

Same as on gitlab, this times out shortly after outputting
"sh: 1: exec: ./storage-daemon/qemu-storage-daemon: not found"

Manually running qos-test, the same way check-qtest-i386 invokes it:

$ QTEST_QEMU_BINARY=./qemu-system-i386 
QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon 
tests/qtest/qos-test --tap -k -m quick < /dev/null

# starting vhost-user backend: exec ./storage-daemon/qemu-storage-daemon 
--blockdev driver=file,node-name=disk0,filename=qtest.XRAzzu --export 
type=vhost-user-blk,id=disk0,addr.type=unix,addr.path=/tmp/qtest-94561-sock.NdKWpt,node-name=disk0,writable=on,num-queues=1
sh: 1: exec: ./storage-daemon/qemu-storage-daemon: not found


The error is different from the previous issue of intermittent hang.
This time the hang is caused by missing qemu-storage-daemon and I
guess the hang could happen reliably each time. The reason of missing 
qemu-storage-daemon is the test doesn't add qemu-storage-daemon as
dependency. If we run `make`, qemu-storage-daemon would be built. But if 
`make check-qtest-i386` is run directly, qemu-storage-daemon wouldn't be 
built. I'll send a patch to make vhost-user-blk test depends on
emu-storage-daemon. 


# starting QEMU: exec ./qemu-system-i386 -qtest unix:/tmp/qtest-94561.sock 
-qtest-log /dev/null -chardev socket,path=/tmp/qtest-94561.qmp,id=char0 -mon 
chardev=char0,mode=control -display none -M pc  -device 
vhost-user-blk-pci,id=drv0,chardev=char1,addr=4.0 -object 
memory-backend-memfd,id=mem,size=256M,share=on  -M memory-backend=mem -m 256M 
-chardev socket id=char1,path=/tmp/qtest-94561-sock.NdKWpt  -accel qtest

*timeout*


vhost-user timing out in realize I suspect. I see that as
an intermittent hang in non-sanitizer configs.

vhost-user folk: Can we either look at fixing this or else disable
the test ? (Stack backtraces available in the other email thread.)

thanks
-- PMM





--
Best regards,
Coiby

Re: [PATCH 03/19] target/ppc: add exclusive user write function for PMU regs

[Richard Henderson]

On 8/9/21 5:29 PM, David Gibson wrote:

  ctx->spr_cb = env->spr_cb;
+ctx->spr = env->spr;


Eep... with that one line you're copying 8kiB of data into the context
structure.  That sounds undesirable.. especially since it look like
you only check 8 bytes of it.

Plus.. TBH, I'm a bit fuzzy on how the disascontext stuff works, but
I'm not sure copying the stuff here is correct.


It isn't.


I think instead you need to actually generate the instructions to read
from MMCR0 and conditionally generate an exception if the permission
bit isn't set.


Or copy exactly the bits you need from MMCR0 for the permission check into env->hflags, so 
that you can later read them from ctx->flags.  Bearing in mind that hflags has only 32 
bits, 19 of them are currently in use.



r~

RE: [PATCH v3 1/3] target-arm: cpu64: Add support for Fujitsu A64FX

[ishii.shuuic...@fujitsu.com]

Thank you very much for the source review and the patch.
We will create a series of V4 patches based on your comments.

Best regards.

> -Original Message-
> From: Andrew Jones 
> Sent: Tuesday, August 10, 2021 8:41 PM
> To: Ishii, Shuuichirou/ 
> Cc: peter.mayd...@linaro.org; qemu-...@nongnu.org; qemu-devel@nongnu.org
> Subject: Re: [PATCH v3 1/3] target-arm: cpu64: Add support for Fujitsu A64FX
> 
> On Tue, Aug 10, 2021 at 08:23:39AM +, ishii.shuuic...@fujitsu.com wrote:
> >
> > Thanks for your comments.
> >
> > Before reposting the fix patch series, based on your comments and the
> > v3 1/3 patch, we have considered the following fixes.
> >
> > If you have any comments on the fixes, please let us know.
> >
> > ---
> >
> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h index
> > 9f0a5f84d5..84ebca731a 100644
> > --- a/target/arm/cpu.h
> > +++ b/target/arm/cpu.h
> > @@ -2145,6 +2145,7 @@ enum arm_features {
> >  ARM_FEATURE_M_SECURITY, /* M profile Security Extension */
> >  ARM_FEATURE_M_MAIN, /* M profile Main Extension */
> >  ARM_FEATURE_V8_1M, /* M profile extras only in v8.1M and later */
> > +ARM_FEATURE_A64FX, /* Fujitsu A64FX processor */
> >  };
> >
> >  static inline int arm_feature(CPUARMState *env, int feature) diff
> > --git a/target/arm/cpu64.c b/target/arm/cpu64.c index
> > 612644941b..62dcb6a919 100644
> > --- a/target/arm/cpu64.c
> > +++ b/target/arm/cpu64.c
> > @@ -248,6 +248,21 @@ static void aarch64_a72_initfn(Object *obj)
> >  define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);  }
> >
> > +static void a64fx_cpu_set_sve(ARMCPU *cpu) {
> > +/* Suppport of A64FX's vector length are 128,256 and 512byte only
> > +*/
> 
> Missing spaces in text and s/byte/bit/
> 
> > +bitmap_zero(cpu->sve_vq_map, ARM_MAX_VQ);
> > +bitmap_zero(cpu->sve_vq_init, ARM_MAX_VQ);
> > +set_bit(0, cpu->sve_vq_map); /* 128byte */
> > +set_bit(0, cpu->sve_vq_init);
> > +set_bit(1, cpu->sve_vq_map); /* 256byte */
> > +set_bit(1, cpu->sve_vq_init);
> > +set_bit(3, cpu->sve_vq_map); /* 512byte */
> > +set_bit(3, cpu->sve_vq_init);
> 
> For all the comments in the above function s/byte/bit/
> 
> > +cpu->sve_max_vq = find_last_bit(cpu->sve_vq_map, ARM_MAX_VQ) +
> 1;
> > +
> 
> Extra blank line
> 
> > +}
> >  void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)  {
> >  /*
> > @@ -448,6 +463,10 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error
> > **errp)
> >
> >  /* From now on sve_max_vq is the actual maximum supported length. */
> >  cpu->sve_max_vq = max_vq;
> > +
> > +   if(arm_feature(&cpu->env, ARM_FEATURE_A64FX)) {
> > +a64fx_cpu_set_sve(cpu);
> > +}
> 
> Bad indentation and spacing, but I don't think this is the right place for 
> this. I
> wouldn't even let ARM_FEATURE_A64FX enter arm_cpu_sve_finalize, since we
> know it doesn't support sve cpu properties.
> While it's ugly wherever we put it, since we have to special case it, I think 
> it's less
> ugly at the callsite
> 
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c index
> 2866dd765882..225800ec361c 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1350,10 +1350,14 @@ void arm_cpu_finalize_features(ARMCPU *cpu, Error
> **errp)
>  Error *local_err = NULL;
> 
>  if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
> -arm_cpu_sve_finalize(cpu, &local_err);
> -if (local_err != NULL) {
> -error_propagate(errp, local_err);
> -return;
> +if (arm_feature(&cpu->env, ARM_FEATURE_A64FX)) {
> +a64fx_cpu_set_sve(cpu);
> +} else {
> +arm_cpu_sve_finalize(cpu, &local_err);
> +if (local_err != NULL) {
> +error_propagate(errp, local_err);
> +return;
> +}
>  }
> 
>  /*
> 
> >  }
> >
> >  static void cpu_max_get_sve_max_vq(Object *obj, Visitor *v, const
> > char *name, @@ -852,6 +871,7 @@ static void aarch64_a64fx_initfn(Object
> *obj)
> >  ARMCPU *cpu = ARM_CPU(obj);
> >
> >  cpu->dtb_compatible = "arm,a64fx";
> > +set_feature(&cpu->env, ARM_FEATURE_A64FX);
> >  set_feature(&cpu->env, ARM_FEATURE_V8);
> >  set_feature(&cpu->env, ARM_FEATURE_NEON);
> >  set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER); @@ -884,10
> > +904,6 @@ static void aarch64_a64fx_initfn(Object *obj)
> >  cpu->gic_vpribits = 5;
> >  cpu->gic_vprebits = 5;
> >  /* TODO:  Add A64FX specific HPC extension registers */
> > -
> > -aarch64_add_sve_properties(obj);
> > -object_property_add(obj, "sve-max-vq", "uint32",
> cpu_max_get_sve_max_vq,
> > -cpu_max_set_sve_max_vq, NULL, NULL);
> >  }
> >
> >  static const ARMCPUInfo aarch64_cpus[] = {
> 
> Otherwise looks OK to me.
> 
> Thanks,
> drew
> 
> >
> > ---
> >
> > Best regards.
> >
> >
> > > -Original Message-
> > > From: Andrew Jones 
> > > Sent: Thursday, August 5, 2021 8:25 PM
> > > To: Ishii, Shuuichirou 
> 

Re: [PATCH for 6.2 39/49] bsd-user: Need to reset CPU after creation.

[Richard Henderson]

On 8/10/21 12:40 PM, Warner Losh wrote:

Slightly amusing that the line was removed in patch 1, because it was 
ifdefed.

Worth folding?


Might as well.


r~

Re: [PATCH 09/10] hw/misc: Add Infineon DPS310 sensor model

[Corey Minyard]

On Mon, Aug 09, 2021 at 03:15:55PM +0200, Cédric Le Goater wrote:
> From: Joel Stanley 
> 
> This contains some hardcoded register values that were obtained from the
> hardware after reading the temperature.
> 
> It does enough to test the Linux kernel driver. The FIFO mode, IRQs and
> operation modes other than the default as used by Linux are not modelled.
> 
> Signed-off-by: Joel Stanley 
> [ clg: Fix sequential reading ]
> Message-Id: <20210616073358.750472-2-j...@jms.id.au>
> Signed-off-by: Cédric Le Goater 
> Message-Id: <20210629142336.750058-4-...@kaod.org>
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/misc/dps310.c| 227 

Can this go into hw/sensor?

-corey

>  hw/arm/Kconfig  |   1 +
>  hw/misc/Kconfig |   4 +
>  hw/misc/meson.build |   1 +
>  4 files changed, 233 insertions(+)
>  create mode 100644 hw/misc/dps310.c
> 
> diff --git a/hw/misc/dps310.c b/hw/misc/dps310.c
> new file mode 100644
> index ..893521ab8516
> --- /dev/null
> +++ b/hw/misc/dps310.c
> @@ -0,0 +1,227 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2017-2021 Joel Stanley , IBM Corporation
> + *
> + * Infineon DPS310 temperature and humidity sensor
> + *
> + * 
> https://www.infineon.com/cms/en/product/sensor/pressure-sensors/pressure-sensors-for-iot/dps310/
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "hw/hw.h"
> +#include "hw/i2c/i2c.h"
> +#include "qapi/error.h"
> +#include "qapi/visitor.h"
> +#include "migration/vmstate.h"
> +
> +#define NUM_REGISTERS   0x33
> +
> +typedef struct DPS310State {
> +/*< private >*/
> +I2CSlave i2c;
> +
> +/*< public >*/
> +uint8_t regs[NUM_REGISTERS];
> +
> +uint8_t len;
> +uint8_t pointer;
> +
> +} DPS310State;
> +
> +#define TYPE_DPS310 "dps310"
> +#define DPS310(obj) OBJECT_CHECK(DPS310State, (obj), TYPE_DPS310)
> +
> +#define DPS310_PRS_B2   0x00
> +#define DPS310_PRS_B1   0x01
> +#define DPS310_PRS_B0   0x02
> +#define DPS310_TMP_B2   0x03
> +#define DPS310_TMP_B1   0x04
> +#define DPS310_TMP_B0   0x05
> +#define DPS310_PRS_CFG  0x06
> +#define DPS310_TMP_CFG  0x07
> +#define  DPS310_TMP_RATE_BITS   (0x70)
> +#define DPS310_MEAS_CFG 0x08
> +#define  DPS310_MEAS_CTRL_BITS  (0x07)
> +#define   DPS310_PRESSURE_ENBIT(0)
> +#define   DPS310_TEMP_ENBIT(1)
> +#define   DPS310_BACKGROUND BIT(2)
> +#define  DPS310_PRS_RDY BIT(4)
> +#define  DPS310_TMP_RDY BIT(5)
> +#define  DPS310_SENSOR_RDY  BIT(6)
> +#define  DPS310_COEF_RDYBIT(7)
> +#define DPS310_CFG_REG  0x09
> +#define DPS310_RESET0x0c
> +#define  DPS310_RESET_MAGIC (BIT(0) | BIT(3))
> +#define DPS310_COEF_BASE0x10
> +#define DPS310_COEF_LAST0x21
> +#define DPS310_COEF_SRC 0x28
> +
> +static void dps310_reset(DeviceState *dev)
> +{
> +DPS310State *s = DPS310(dev);
> +
> +static const uint8_t regs_reset_state[] = {
> +0xfe, 0x2f, 0xee, 0x02, 0x69, 0xa6, 0x00, 0x80, 0xc7, 0x00, 0x00, 
> 0x00,
> +0x00, 0x10, 0x00, 0x00, 0x0e, 0x1e, 0xdd, 0x13, 0xca, 0x5f, 0x21, 
> 0x52,
> +0xf9, 0xc6, 0x04, 0xd1, 0xdb, 0x47, 0x00, 0x5b, 0xfb, 0x3a, 0x00, 
> 0x00,
> +0x20, 0x49, 0x4e, 0xa5, 0x90, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
> 0x00,
> +0x60, 0x15, 0x02
> +};
> +
> +QEMU_BUILD_BUG_ON(sizeof(regs_reset_state) != sizeof(s->regs));
> +
> +memcpy(s->regs, regs_reset_state, sizeof(s->regs));
> +s->pointer = 0;
> +
> +/* TODO: assert these after some timeout ? */
> +s->regs[DPS310_MEAS_CFG] = DPS310_COEF_RDY | DPS310_SENSOR_RDY
> +| DPS310_TMP_RDY | DPS310_PRS_RDY;
> +}
> +
> +static uint8_t dps310_read(DPS310State *s, uint8_t reg)
> +{
> +if (reg >= sizeof(s->regs)) {
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: register 0x%02x out of bounds\n",
> +  __func__, s->pointer);
> +return 0xFF;
> +}
> +
> +switch (reg) {
> +case DPS310_PRS_B2:
> +case DPS310_PRS_B1:
> +case DPS310_PRS_B0:
> +case DPS310_TMP_B2:
> +case DPS310_TMP_B1:
> +case DPS310_TMP_B0:
> +case DPS310_PRS_CFG:
> +case DPS310_TMP_CFG:
> +case DPS310_MEAS_CFG:
> +case DPS310_CFG_REG:
> +case DPS310_COEF_BASE...DPS310_COEF_LAST:
> +case DPS310_COEF_SRC:
> +case 0x32: /* Undocumented register to indicate workaround not required 
> */
> +return s->regs[reg];
> +default:
> +qemu_log_mask(LOG_UNIMP, "%s: register 0x%02x unimplemented\n",
> +  __func__, reg);
> +return 0xFF;
> +}
> +}
> +
> +static void dps310_write(DPS310State *s, uint8_t reg, uint8_t data)
> +{
> +if (reg >= sizeof(s->regs)) {
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: register %d out of bounds\n",
> +  __func__, s->pointer);
> +return;
> +}
> +
> +switch (reg) {
>

Re: [PATCH for 6.2 37/49] bsd-user: update debugging in mmap.c

[Richard Henderson]

On 8/10/21 12:34 PM, Warner Losh wrote:

For the to-do list: convert to qemu_log_mask or tracepoints.


Will do. Should I create a bsd-user/TODO file with all these in it (or rather, 
should I
upstream the one I've already created)?


Could do.  Dunno if its easier or not, but we also have a set of Issues with the "Bite 
Sized" tag for small cleanups like this.



r~

[PATCH for-6.1] tcg/i386: Split P_VEXW from P_REXW

[Richard Henderson]

We need to be able to represent VEX.W on a 32-bit host, where REX.W
will always be zero.  Fixes the encoding for VPSLLVQ and VPSRLVQ.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/385
Signed-off-by: Richard Henderson 
---
 tcg/i386/tcg-target.c.inc | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc
index 98d924b91a..997510109d 100644
--- a/tcg/i386/tcg-target.c.inc
+++ b/tcg/i386/tcg-target.c.inc
@@ -241,8 +241,9 @@ static bool tcg_target_const_match(int64_t val, TCGType 
type, int ct)
 #define P_EXT  0x100   /* 0x0f opcode prefix */
 #define P_EXT38 0x200   /* 0x0f 0x38 opcode prefix */
 #define P_DATA160x400   /* 0x66 opcode prefix */
+#define P_VEXW  0x1000  /* Set VEX.W = 1 */
 #if TCG_TARGET_REG_BITS == 64
-# define P_REXW 0x1000  /* Set REX.W = 1 */
+# define P_REXW P_VEXW  /* Set REX.W = 1; match VEXW */
 # define P_REXB_R   0x2000  /* REG field as byte register */
 # define P_REXB_RM  0x4000  /* R/M field as byte register */
 # define P_GS   0x8000  /* gs segment override */
@@ -410,13 +411,13 @@ static bool tcg_target_const_match(int64_t val, TCGType 
type, int ct)
 #define OPC_VPBROADCASTW (0x79 | P_EXT38 | P_DATA16)
 #define OPC_VPBROADCASTD (0x58 | P_EXT38 | P_DATA16)
 #define OPC_VPBROADCASTQ (0x59 | P_EXT38 | P_DATA16)
-#define OPC_VPERMQ  (0x00 | P_EXT3A | P_DATA16 | P_REXW)
+#define OPC_VPERMQ  (0x00 | P_EXT3A | P_DATA16 | P_VEXW)
 #define OPC_VPERM2I128  (0x46 | P_EXT3A | P_DATA16 | P_VEXL)
 #define OPC_VPSLLVD (0x47 | P_EXT38 | P_DATA16)
-#define OPC_VPSLLVQ (0x47 | P_EXT38 | P_DATA16 | P_REXW)
+#define OPC_VPSLLVQ (0x47 | P_EXT38 | P_DATA16 | P_VEXW)
 #define OPC_VPSRAVD (0x46 | P_EXT38 | P_DATA16)
 #define OPC_VPSRLVD (0x45 | P_EXT38 | P_DATA16)
-#define OPC_VPSRLVQ (0x45 | P_EXT38 | P_DATA16 | P_REXW)
+#define OPC_VPSRLVQ (0x45 | P_EXT38 | P_DATA16 | P_VEXW)
 #define OPC_VZEROUPPER  (0x77 | P_EXT)
 #define OPC_XCHG_ax_r32(0x90)
 
@@ -576,7 +577,7 @@ static void tcg_out_vex_opc(TCGContext *s, int opc, int r, 
int v,
 
 /* Use the two byte form if possible, which cannot encode
VEX.W, VEX.B, VEX.X, or an m- field other than P_EXT.  */
-if ((opc & (P_EXT | P_EXT38 | P_EXT3A | P_REXW)) == P_EXT
+if ((opc & (P_EXT | P_EXT38 | P_EXT3A | P_VEXW)) == P_EXT
 && ((rm | index) & 8) == 0) {
 /* Two byte VEX prefix.  */
 tcg_out8(s, 0xc5);
@@ -601,7 +602,7 @@ static void tcg_out_vex_opc(TCGContext *s, int opc, int r, 
int v,
 tmp |= (rm & 8 ? 0 : 0x20);/* VEX.B */
 tcg_out8(s, tmp);
 
-tmp = (opc & P_REXW ? 0x80 : 0);   /* VEX.W */
+tmp = (opc & P_VEXW ? 0x80 : 0);   /* VEX.W */
 }
 
 tmp |= (opc & P_VEXL ? 0x04 : 0);  /* VEX.L */
-- 
2.25.1

Re: [PATCH 06/19] target/ppc/pmu_book3s_helper: enable PMC1-PMC4 events

[Daniel Henrique Barboza]

On 8/10/21 12:03 PM, Daniel Henrique Barboza wrote:



On 8/10/21 12:42 AM, David Gibson wrote:

On Mon, Aug 09, 2021 at 10:10:44AM -0300, Daniel Henrique Barboza wrote:

So far the PMU logic was using PMC5 for instruction counting (linux
kernel PM_INST_CMPL) and PMC6 to count cycles (PM_CYC). We aren't using
PMCs 1-4.

Let's enable all PMCs to count these 2 events we already provide. The
logic used to calculate PMC5 is now being provided by
update_PMC_PM_INST_CMPL() and PMC6 logic is now implemented in
update_PMC_PM_CYC().

The enablement of these 2 events for all PMUs are done by using the
Linux kernel definition of those events: 0x02 for PM_INST_CMPL and 0x1e
for PM_CYC,


I'm confused by this.  Surely the specific values here should be
defined by the hardware, not by Linux.


The hardware/PowerISA defines these events as follows for all counters:

00 Disable events. (No events occur.)
01-BF Implementation-dependent
C0-DF Reserved

And then hardware events defined by the ISA goes from E0 to FF. Each counter
has a different set of hardware defined events in this range.

The Linux perf driver defines some events in the 'Implementation-dependent'
area, allowing for events codes such as '0x02' to count instructions
completed in PMC1 even though this event is not defined in the ISA for this
PMC. I am assuming that the real hardware - at least the ones that IBM
produces - does this mapping internally. I'll ask around to see if I find
out whether it's the HW or some part of the Perf subsystem that I don't
comprehend that are doing it.


The kernel guys confirmed that the HW is aware of the implementantion-dependent
Perf event codes that the Linux kernel uses. Also, by reading the kernel code it
is safe to say that this is true since Power7 at least.

What we can do here to play nicer with other non-IBM PowerPC chips, that might
not have the same implementation-dependent Perf events in the hardware, is to 
make
this mapping only for emulation of IBM processors. That way we can emulate these
events that IBM PMU implements while not making any assumptions for every other
PowerPC chip that implements Book3s.


Thanks,


Daniel





I am not particularly happy about having to rely on 'implementation-dependent'
fields that are defined by the Perf subsystem of Linux in the emulator
code that should be OS-agnostic. Unfortunately, I didn't find any alternative
to make the kernel operate this PMU implementation other than baking these
event codes into the PMU logic.


Thanks,


Daniel





all of those defined by specific bits in MMCR1 for each PMC.
PMCs 1-4 relies on the correct event to be defined in MMCR1. PMC5 and
PMC6 will count PM_INST_CMPL and PMC_CYC, respectively, regardless of
MMCR1 setup.

Signed-off-by: Daniel Henrique Barboza 
---
  target/ppc/cpu.h   |  8 +
  target/ppc/pmu_book3s_helper.c | 60 --
  2 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 8cea8f2aca..afd9cd402b 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -350,6 +350,14 @@ typedef struct ppc_v3_pate_t {
  #define MMCR0_FCECE PPC_BIT(38) /* FC on Enabled Cond or Event */
  #define MMCR0_PMCC  PPC_BITMASK(44, 45) /* PMC Control */
+#define MMCR1_PMC1SEL_SHIFT (63 - 39)
+#define MMCR1_PMC1SEL PPC_BITMASK(32, 39)
+#define MMCR1_PMC2SEL_SHIFT (63 - 47)
+#define MMCR1_PMC2SEL PPC_BITMASK(40, 47)
+#define MMCR1_PMC3SEL_SHIFT (63 - 55)
+#define MMCR1_PMC3SEL PPC_BITMASK(48, 55)
+#define MMCR1_PMC4SEL PPC_BITMASK(56, 63)
+
  /* LPCR bits */
  #define LPCR_VPM0 PPC_BIT(0)
  #define LPCR_VPM1 PPC_BIT(1)
diff --git a/target/ppc/pmu_book3s_helper.c b/target/ppc/pmu_book3s_helper.c
index 0994531f65..99e62bd37b 100644
--- a/target/ppc/pmu_book3s_helper.c
+++ b/target/ppc/pmu_book3s_helper.c
@@ -28,6 +28,56 @@ static uint64_t get_cycles(uint64_t insns)
  return insns * 4;
  }
+static void update_PMC_PM_INST_CMPL(CPUPPCState *env, int sprn,
+    uint64_t curr_icount)
+{
+    env->spr[sprn] += curr_icount - env->pmu_base_icount;
+}
+
+static void update_PMC_PM_CYC(CPUPPCState *env, int sprn,
+  uint64_t curr_icount)
+{
+    uint64_t insns = curr_icount - env->pmu_base_icount;
+    env->spr[sprn] += get_cycles(insns);
+}
+
+static void update_programmable_PMC_reg(CPUPPCState *env, int sprn,
+    uint64_t curr_icount)
+{
+    int event;
+
+    switch (sprn) {
+    case SPR_POWER_PMC1:
+    event = MMCR1_PMC1SEL & env->spr[SPR_POWER_MMCR1];
+    event = event >> MMCR1_PMC1SEL_SHIFT;
+    break;
+    case SPR_POWER_PMC2:
+    event = MMCR1_PMC2SEL & env->spr[SPR_POWER_MMCR1];
+    event = event >> MMCR1_PMC2SEL_SHIFT;
+    break;
+    case SPR_POWER_PMC3:
+    event = MMCR1_PMC3SEL & env->spr[SPR_POWER_MMCR1];
+    event = event >> MMCR1_PMC3SEL_SHIFT;
+    break;
+    case SPR_POWER_PMC4:
+   

Re: [PATCH for 6.2 39/49] bsd-user: Need to reset CPU after creation.

[Warner Losh]

On Tue, Aug 10, 2021 at 10:32 AM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 8/7/21 11:42 AM, Warner Losh wrote:
> > From: Warner Losh
> >
> > Signed-off-by: Warner Losh
> > ---
> >   bsd-user/main.c | 2 ++
> >   1 file changed, 2 insertions(+)
>
> Reviewed-by: Richard Henderson 
>
> Slightly amusing that the line was removed in patch 1, because it was
> ifdefed.
>

Worth folding?

And good eye!

Warner

Re: [PATCH for 6.2 38/49] bsd-user: Update mapping to handle reserved and starting conditions

[Warner Losh]

On Tue, Aug 10, 2021 at 10:27 AM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 8/7/21 11:42 AM, Warner Losh wrote:
> > From: Warner Losh 
> >
> > Update the reserved base based on what platform we're on, as well as the
> > start of the mmap range. Update routines that find va ranges to interact
> > with the reserved ranges as well as properly align the mapping (this is
> > especially important for targets whose page size does not match the
> > host's). Loop where appropriate when the initial address space offered
> > by mmap does not meet the contraints.
> >
> > Signed-off-by: Mikaël Urankar 
> > Signed-off-by: Stacey Son 
> > Signed-off-by: Warner Losh 
> > ---
> >   bsd-user/main.c |  23 ++-
> >   bsd-user/mmap.c | 372 
> >   bsd-user/qemu.h |   5 +-
> >   3 files changed, 335 insertions(+), 65 deletions(-)
> >
> > diff --git a/bsd-user/main.c b/bsd-user/main.c
> > index 93ef9298b8..36852604f8 100644
> > --- a/bsd-user/main.c
> > +++ b/bsd-user/main.c
> > @@ -49,12 +49,29 @@
> >   #include "target_arch_cpu.h"
> >
> >   int singlestep;
> > -unsigned long mmap_min_addr;
> >   uintptr_t guest_base;
> >   static const char *cpu_model;
> >   static const char *cpu_type;
> >   bool have_guest_base;
> > +#if (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
> > +/*
> > + * When running 32-on-64 we should make sure we can fit all of the
> possible
> > + * guest address space into a contiguous chunk of virtual host memory.
> > + *
> > + * This way we will never overlap with our own libraries or binaries or
> stack
> > + * or anything else that QEMU maps.
> > + */
> > +# ifdef TARGET_MIPS
> > +/* MIPS only supports 31 bits of virtual address space for user space */
> > +unsigned long reserved_va = 0x7700;
> > +# elif defined(TARGET_PPC64)
> > +unsigned long reserved_va = 0xf000;
> > +# else
> > +unsigned long reserved_va = 0xf700;
> > +# endif
> > +#else
> >   unsigned long reserved_va;
> > +#endif
>
> All of these 7's look to be copying an old linux-user bug.
> I cleaned this up in 18e80c55bb6.
>

Oh nice. I'd missed that one... a bit before I started hacking on bsd-user,
but quite nice indeed.


> Otherwise,
> Acked-by: Richard Henderson 
>
> I would hope one day this memory management could be shared between the
> user-only
> implementations.  It's complicated enough...
>

Honestly, one reason I want to get "caught up" with upstream is for such a
refactor. There's
so much duplication that it's hard to keep up.

Warner

Re: [PATCH for 6.2 37/49] bsd-user: update debugging in mmap.c

[Warner Losh]

On Tue, Aug 10, 2021 at 10:19 AM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 8/7/21 11:42 AM, Warner Losh wrote:
> > From: Warner Losh
> >
> > Update the debugging code for new features and different targets.
> >
> > Signed-off-by: Mikaël Urankar
> > Signed-off-by: Sean Bruno
> > Signed-off-by: Kyle Evans
> > Signed-off-by: Warner Losh
> > ---
> >   bsd-user/mmap.c | 49 ++---
> >   1 file changed, 30 insertions(+), 19 deletions(-)
>
> Acked-by: Richard Henderson 
>

Thanks


> For the to-do list: convert to qemu_log_mask or tracepoints.
>

Will do. Should I create a bsd-user/TODO file with all these in it (or
rather, should I
upstream the one I've already created)?

Warner

Re: [PATCH for 6.2 35/49] bsd-user: remove error_init

[Warner Losh]

On Tue, Aug 10, 2021 at 9:07 AM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 8/7/21 11:42 AM, Warner Losh wrote:
> > From: Warner Losh 
> >
> > Error reporting isn't used, so gc it until it's used.
> >
> > Signed-off-by: Warner Losh  
>
> That's not true.  At minimum, tcg/ uses Error during alloc_code_gen_buffer.
>

I'm not sure how I overlooked that, so I'll just drop this patch. Thanks
for the
good eye.

Warner


> r~
>
> > ---
> >   bsd-user/main.c | 2 --
> >   1 file changed, 2 deletions(-)
> >
> > diff --git a/bsd-user/main.c b/bsd-user/main.c
> > index b5527537b4..7e20430fb7 100644
> > --- a/bsd-user/main.c
> > +++ b/bsd-user/main.c
> > @@ -34,7 +34,6 @@
> >   #include "qapi/error.h"
> >   #include "qemu.h"
> >   #include "qemu/config-file.h"
> > -#include "qemu/error-report.h"
> >   #include "qemu/path.h"
> >   #include "qemu/help_option.h"
> >   #include "qemu/module.h"
> > @@ -223,7 +222,6 @@ int main(int argc, char **argv)
> >
> >   save_proc_pathname(argv[0]);
> >
> > -error_init(argv[0]);
> >   module_call_init(MODULE_INIT_TRACE);
> >   qemu_init_cpu_list();
> >   module_call_init(MODULE_INIT_QOM);
> >
>
>

Re: [PATCH for 6.2 34/49] bsd-user: Fix initializtion of task state

[Warner Losh]

On Tue, Aug 10, 2021 at 9:03 AM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 8/7/21 11:42 AM, Warner Losh wrote:
> > @@ -459,21 +435,11 @@ int main(int argc, char **argv)
> >   qemu_log("entry   0x" TARGET_ABI_FMT_lx "\n", info->entry);
> >   }
> >
> > -target_set_brk(info->brk);
> > -syscall_init();
> > -signal_init();
> > -
> > -/*
> > - * Now that we've loaded the binary, GUEST_BASE is fixed.  Delay
> > - * generating the prologue until now so that the prologue can take
> > - * the real value of GUEST_BASE into account.
> > - */
> > -tcg_prologue_init(tcg_ctx);
> > -
> >   /* build Task State */
> > -memset(ts, 0, sizeof(TaskState));
> > +ts = g_new0(TaskState, 1);
> >   init_task_state(ts);
> >   ts->info = info;
> > +ts->bprm = &bprm;
> >   cpu->opaque = ts;
> >
> >   target_set_brk(info->brk);
>
> It looks like some of this damage occurs in patch 22
> ("bsd-user: Move per-cpu code into target_arch_cpu.h")
> and could reasonably be squashed back.
>
> Otherwise,
> Reviewed-by: Richard Henderson 
>

I took the easy way and folded them together. Thanks for the tip.

Warner

[ANNOUNCE] QEMU 6.1.0-rc3 is now available

[Michael Roth]

Hello,

On behalf of the QEMU Team, I'd like to announce the availability of the
fourth release candidate for the QEMU 6.1 release.  This release is meant
for testing purposes and should not be used in a production environment.

  http://download.qemu-project.org/qemu-6.1.0-rc3.tar.xz
  http://download.qemu-project.org/qemu-6.1.0-rc3.tar.xz.sig

A note from the maintainer:

  If there are no show-stopper issues found in this release candidate,
  the final 6.1.0 release should be next week, on the 17th. If we need
  to roll an rc4 we'll probably release that on the 17th and do the
  final release the week after.

You can help improve the quality of the QEMU 6.1 release by testing this
release and reporting bugs using our GitLab issue tracker:

  https://gitlab.com/qemu-project/qemu/-/issues

The release plan, as well a documented known issues for release
candidates, are available at:

  http://wiki.qemu.org/Planning/6.1

Please add entries to the ChangeLog for the 6.1 release below:

  http://wiki.qemu.org/ChangeLog/6.1

Thank you to everyone involved!

Changes since rc3:

703e8cd618: Update version for v6.1.0-rc3 release (Peter Maydell)
b0c4798f97: MAINTAINERS: Name and email address change (Hanna Reitz)
6ff5b5d6d5: ui/sdl2: Check return value from g_setenv() (Peter Maydell)
da77adbaf6: audio: Never send migration section (Dr. David Alan Gilbert)
7bce330ae4: ui/gtk: retry sending VTE console input (Volker Rümelin)
057489dd15: qga: fix leak of base64 decoded data on command error (Daniel P. 
Berrangé)
a6d2bb25cf: tests: filter out TLS distinguished name in certificate checks 
(Daniel P. Berrangé)
50482fda98: block/export/fuse.c: fix musl build (Fabrice Fontaine)
5f4884c441: hw/nvme: fix missing variable initializers (Klaus Jensen)
abc14fd056: meson: fix logic for gnutls check (Alyssa Ross)
a68403b0a6: chardev: report a simpler error about duplicated id (Marc-André 
Lureau)
64195b0d36: chardev: give some context on chardev-add error (Marc-André Lureau)
733ba02084: chardev: fix qemu_chr_open_fd() with fd_in==fd_out (Marc-André 
Lureau)
46fe3ff6ea: chardev: fix qemu_chr_open_fd() being called with fd=-1 (Marc-André 
Lureau)
bb2b058f1a: chardev: fix fd_chr_add_watch() when in != out (Marc-André Lureau)
bf7b1eab25: chardev: mark explicitly first argument as poisoned (Marc-André 
Lureau)
030912e01c: linux-user/elfload: byteswap i386 registers when dumping core (Ilya 
Leoshkevich)
0c40c18ecd: linux-user: fix guest/host address mixup in i386 setup_rt_frame() 
(Ilya Leoshkevich)
30f80be34b: chardev/socket: print a more correct command-line address 
(Marc-André Lureau)
4cfd970ec1: util: fix abstract socket path copy (Marc-André Lureau)
68e6dc594a: docs: convert writing-qmp-commands.txt to writing-qmp-commands.rst 
(John Snow)
9c66762a60: docs/qapi-code-gen: add cross-references (John Snow)
55927c5f32: docs/qapi-code-gen: Beautify formatting (John Snow)
f7aa076dbd: docs: convert qapi-code-gen.txt to ReST (John Snow)
e0366f9f2b: docs/devel/qapi-code-gen: Update examples to match current code 
(Markus Armbruster)

Re: [PATCH 15/19] target/ppc/pmu_book3s_helper: enable counter negative for all PMCs

[Daniel Henrique Barboza]

On 8/10/21 1:11 AM, David Gibson wrote:

On Mon, Aug 09, 2021 at 10:10:53AM -0300, Daniel Henrique Barboza wrote:

All performance monitor counters can trigger a counter negative
condition if the proper MMCR0 bits are set. This patch does that by
doing the following:

- pmc_counter_negative_enabled() will check whether a given PMC is
eligible to trigger the counter negative alert;

- get_counter_neg_timeout() will return the timeout for the counter
negative condition for a given PMC, or -1 if the PMC is not able to
trigger this alert;

- the existing counter_negative_cond_enabled() now must consider the
counter negative bit for PMCs 2-6, MMCR0_PMCjCE;

- set_PMU_excp_timer() will now search all existing PMCs for the
shortest counter negative timeout. The shortest timeout will be used to
set the PMC interrupt timer.

This change makes most EBB powepc kernel tests pass, validating that the
existing EBB logic is consistent. There are a few tests that aren't passing
due to additional PMU bits and perf events that aren't covered yet.
We'll attempt to cover some of those in the next patches.

Signed-off-by: Daniel Henrique Barboza 
---
  target/ppc/cpu.h   |  1 +
  target/ppc/pmu_book3s_helper.c | 96 ++
  2 files changed, 87 insertions(+), 10 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 5c81d459f4..1aa1fd42af 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -351,6 +351,7 @@ typedef struct ppc_v3_pate_t {
  #define MMCR0_FCECE PPC_BIT(38) /* FC on Enabled Cond or Event */
  #define MMCR0_PMCC  PPC_BITMASK(44, 45) /* PMC Control */
  #define MMCR0_PMC1CE PPC_BIT(48)
+#define MMCR0_PMCjCE PPC_BIT(49)
  
  #define MMCR1_PMC1SEL_SHIFT (63 - 39)

  #define MMCR1_PMC1SEL PPC_BITMASK(32, 39)
diff --git a/target/ppc/pmu_book3s_helper.c b/target/ppc/pmu_book3s_helper.c
index 7126e9b3d5..c5c5ab38c9 100644
--- a/target/ppc/pmu_book3s_helper.c
+++ b/target/ppc/pmu_book3s_helper.c
@@ -143,22 +143,98 @@ static int64_t get_CYC_timeout(CPUPPCState *env, int sprn)
  return muldiv64(remaining_cyc, NANOSECONDS_PER_SECOND, PPC_CPU_FREQ);
  }
  
-static void set_PMU_excp_timer(CPUPPCState *env)

+static bool pmc_counter_negative_enabled(CPUPPCState *env, int sprn)
  {
-uint64_t timeout, now;
+switch (sprn) {
+case SPR_POWER_PMC1:
+return env->spr[SPR_POWER_MMCR0] & MMCR0_PMC1CE;
  
-if (!(env->spr[SPR_POWER_MMCR0] & MMCR0_PMC1CE)) {

-return;
+case SPR_POWER_PMC2:
+case SPR_POWER_PMC3:
+case SPR_POWER_PMC4:
+case SPR_POWER_PMC5:
+case SPR_POWER_PMC6:
+return env->spr[SPR_POWER_MMCR0] & MMCR0_PMCjCE;
+
+default:
+break;
  }
  
-switch (get_PMC_event(env, SPR_POWER_PMC1)) {

-case 0x2:
-timeout = get_INST_CMPL_timeout(env, SPR_POWER_PMC1);
+return false;
+}
+
+static int64_t get_counter_neg_timeout(CPUPPCState *env, int sprn)
+{
+int64_t timeout = -1;
+
+if (!pmc_counter_negative_enabled(env, sprn)) {
+return -1;
+}
+
+if (env->spr[sprn] >= COUNTER_NEGATIVE_VAL) {
+return 0;
+}
+
+switch (sprn) {
+case SPR_POWER_PMC1:
+case SPR_POWER_PMC2:
+case SPR_POWER_PMC3:
+case SPR_POWER_PMC4:
+switch (get_PMC_event(env, sprn)) {
+case 0x2:
+timeout = get_INST_CMPL_timeout(env, sprn);
+break;
+case 0x1E:
+timeout = get_CYC_timeout(env, sprn);
+break;
+}
+
  break;
-case 0x1e:
-timeout = get_CYC_timeout(env, SPR_POWER_PMC1);
+case SPR_POWER_PMC5:
+timeout = get_INST_CMPL_timeout(env, sprn);
+break;
+case SPR_POWER_PMC6:
+timeout = get_CYC_timeout(env, sprn);
  break;
  default:
+break;
+}
+
+return timeout;
+}
+
+static void set_PMU_excp_timer(CPUPPCState *env)
+{
+int64_t timeout = -1;
+uint64_t now;
+int i;
+
+/*
+ * Scroll through all PMCs and check which one is closer to a
+ * counter negative timeout.


I'm wondering if it would be simpler to use a separate timer for each
PMC: after all the core timer logic must have already implemented this
"who fires first" logic.


The first draft had 6 timers, one for each PMC. It would make this step to
determine the lowest timeout unnecessary.

I gave it up because we would need to pause the running timers of the other
PMCs when the PPC_INTERRUPT_PMC happens with MMCR0_FCECE (frozen counters on
Enabled Condition or Event) set. Resuming the timers back would require to
update the PMCs (which would now have different icount_bases).

For our current usage this single timer approach seems less complicated. And
the ISA allows for multiple/concurrent overflows to be reported at the same
alert:

"An enabled condition or event causes a Perfor-
mance Monitor alert if Performance Monitor alerts
are enabled by the corresponding “Enable” bit in
MMCR0. (..) A single Performance Monitor alert may
r

Re: [PATCH 13/19] target/ppc/translate: PMU: handle setting of PMCs while running

[Daniel Henrique Barboza]

On 8/10/21 1:06 AM, David Gibson wrote:

On Mon, Aug 09, 2021 at 10:10:51AM -0300, Daniel Henrique Barboza wrote:

The initial PMU support were made under the assumption that the counters
would be set before running the PMU and read after either freezing the
PMU manually or via a performance monitor alert.

Turns out that some EBB powerpc kernel tests set the counters after
unfreezing the counters. Setting a PMC value when the PMU is running
means that, at that moment, the baseline for calculating the events (set
in env->pmu_base_icount) needs to be updated. Updating this baseline
means that we need to update all the PMCs with their actual value at
that moment. Any existing counter negative timer needs to be discarded
an a new one, with the updated values, must be set again.

This patch does that via a new 'helper_store_pmc()' that is called in
the mtspr() callbacks of the PMU registers, spr_write_pmu_ureg() and
spr_write_pmu_generic() in target/ppc/translate.c

With this change, EBB powerpc kernel tests such as  'no_handler_test'
are now passing.

Signed-off-by: Daniel Henrique Barboza 
---
  target/ppc/helper.h|  1 +
  target/ppc/pmu_book3s_helper.c | 36 --
  target/ppc/translate.c | 27 +
  3 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 5122632784..757665b360 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -21,6 +21,7 @@ DEF_HELPER_1(hrfid, void, env)
  DEF_HELPER_2(store_lpcr, void, env, tl)
  DEF_HELPER_2(store_pcr, void, env, tl)
  DEF_HELPER_2(store_mmcr0, void, env, tl)
+DEF_HELPER_3(store_pmc, void, env, i32, i64)
  #endif
  DEF_HELPER_1(check_tlb_flush_local, void, env)
  DEF_HELPER_1(check_tlb_flush_global, void, env)
diff --git a/target/ppc/pmu_book3s_helper.c b/target/ppc/pmu_book3s_helper.c
index 58ae65e22b..e7af273cb6 100644
--- a/target/ppc/pmu_book3s_helper.c
+++ b/target/ppc/pmu_book3s_helper.c
@@ -173,7 +173,7 @@ void cpu_ppc_pmu_timer_init(CPUPPCState *env)
  env->pmu_intr_timer = timer;
  }
  
-static bool mmcr0_counter_neg_cond_enabled(uint64_t mmcr0)

+static bool counter_negative_cond_enabled(uint64_t mmcr0)


Can you fold this rename into the patch which introduces the function
please.


  {
  return mmcr0 & MMCR0_PMC1CE;
  }
@@ -219,9 +219,41 @@ void helper_store_mmcr0(CPUPPCState *env, target_ulong 
value)
   * Start performance monitor alert timer for counter negative
   * events, if needed.
   */
-if (mmcr0_counter_neg_cond_enabled(env->spr[SPR_POWER_MMCR0])) {
+if (counter_negative_cond_enabled(env->spr[SPR_POWER_MMCR0])) {
  set_PMU_excp_timer(env);
  }
  }
  }
  }
+
+void helper_store_pmc(CPUPPCState *env, uint32_t sprn, uint64_t value)
+{
+bool pmu_frozen = env->spr[SPR_POWER_MMCR0] & MMCR0_FC;
+uint64_t curr_icount, icount_delta;
+
+if (pmu_frozen) {
+env->spr[sprn] = value;
+return;
+}
+
+curr_icount = (uint64_t)icount_get_raw();
+icount_delta = curr_icount - env->pmu_base_icount;
+
+/* Update the counter with the events counted so far */
+update_PMCs(env, icount_delta);
+
+/* Set the counter to the desirable value after update_PMCs() */
+env->spr[sprn] = value;
+
+/*
+ * Delete the current timer and restart a new one with the
+ * updated values.
+ */
+timer_del(env->pmu_intr_timer);
+
+env->pmu_base_icount = curr_icount;


I'd expect some of this code to be shared with the unfreeze path using
a helper.  Is there a reason that's not the case?

Do you also need to deal with any counter interrupts that have already
been generated by the old counter?  Are the counter overflow events
edge-triggered or level-triggered?



I'd say that counter negative overflow are edge triggered - we can set any
starting value for the counters but the counter negative overflow is
triggered always with the counter reaching 0x800.

If a counter was about to trigger a c.n overflow and then the user set it
back to 0, that c.n overflow is lost.


Thanks,


Daniel





+if (counter_negative_cond_enabled(env->spr[SPR_POWER_MMCR0])) {
+set_PMU_excp_timer(env);
+}
+}
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index afc254a03f..3e890cc4d8 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -409,11 +409,25 @@ void spr_write_generic(DisasContext *ctx, int sprn, int 
gprn)
  
  void spr_write_pmu_generic(DisasContext *ctx, int sprn, int gprn)

  {
+TCGv_i32 t_sprn;
+
  switch (sprn) {
  case SPR_POWER_MMCR0:
  gen_icount_io_start(ctx);
  gen_helper_store_mmcr0(cpu_env, cpu_gpr[gprn]);
  break;
+case SPR_POWER_PMC1:
+case SPR_POWER_PMC2:
+case SPR_POWER_PMC3:
+case SPR_POWER_PMC4:
+case SPR_POWER_PMC5:
+case SPR_POWER_PMC6:
+gen_icount_io_start(ctx

Re: [PATCH] tests/acceptance: Allow overwrite smp and memory

[Ahmed Abouzied]

ping

Just pinging about this little patch. Patchew link here:
https://patchew.org/QEMU/2021080257.50946-1-em...@aabouzied.com/.

Best regards,
Ahmed Abouzied

On Tue, Aug 3, 2021 at 12:24 AM Ahmed Abouzied  wrote:

> Removes the hard-coded values in setUp(). Class inheriting from
> avocado_qemu.LinuxTest can overwrite the default smp and memory instead.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/453
> Signed-off-by: Ahmed Abouzied 
> ---
>  tests/acceptance/avocado_qemu/__init__.py | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py
> b/tests/acceptance/avocado_qemu/__init__.py
> index 2c4fef3e14..2639b89c84 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -441,6 +441,8 @@ class LinuxTest(Test, LinuxSSHMixIn):
>  distro = None
>  username = 'root'
>  password = 'password'
> +smp = '2'
> +memory = '1024'
>
>  def _set_distro(self):
>  distro_name = self.params.get(
> @@ -471,8 +473,8 @@ def _set_distro(self):
>  def setUp(self, ssh_pubkey=None, network_device_type='virtio-net'):
>  super(LinuxTest, self).setUp()
>  self._set_distro()
> -self.vm.add_args('-smp', '2')
> -self.vm.add_args('-m', '1024')
> +self.vm.add_args('-smp', self.smp)
> +self.vm.add_args('-m', self.memory)
>  # The following network device allows for SSH connections
>  self.vm.add_args('-netdev', 'user,id=vnet,hostfwd=:127.0.0.1:0
> -:22',
>   '-device', '%s,netdev=vnet' %
> network_device_type)
> --
> 2.25.1
>
>

Re: [PATCH 12/19] target/ppc/pmu_book3s_helper.c: enable PMC1 counter negative EBB

[Daniel Henrique Barboza]

On 8/10/21 1:01 AM, David Gibson wrote:

On Mon, Aug 09, 2021 at 10:10:50AM -0300, Daniel Henrique Barboza wrote:

This patch starts the counter negative EBB support by enabling PMC1
counter negative condition.

A counter negative condition happens when a performance monitor counter
reaches the value 0x8000. When that happens, if a counter negative
condition is enabled in that counter, a performance monitor alert is
triggered. For PMC1, this condition is enabled by MMCR0_PMC1CE.

An icount-based logic is used to predict when we need to wake up the timer
to trigger the alert in both PM_INST_CMPL (0x2) and PM_CYC (0x1E) events.
The timer callback will then trigger a PPC_INTERRUPT_PMC which will become a
event-based exception later.

Some EBB powerpc kernel selftests are passing after this patch, but a
substancial amount of them relies on other PMCs to be enabled and events
that we don't support at this moment. We'll address that in the next
patches.

Signed-off-by: Daniel Henrique Barboza 
---
  target/ppc/cpu.h   |   1 +
  target/ppc/pmu_book3s_helper.c | 127 +++--
  2 files changed, 92 insertions(+), 36 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 1d38b8cf7a..5c81d459f4 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -350,6 +350,7 @@ typedef struct ppc_v3_pate_t {
  #define MMCR0_EBE   PPC_BIT(43) /* Perf Monitor EBB Enable */
  #define MMCR0_FCECE PPC_BIT(38) /* FC on Enabled Cond or Event */
  #define MMCR0_PMCC  PPC_BITMASK(44, 45) /* PMC Control */
+#define MMCR0_PMC1CE PPC_BIT(48)
  
  #define MMCR1_PMC1SEL_SHIFT (63 - 39)

  #define MMCR1_PMC1SEL PPC_BITMASK(32, 39)
diff --git a/target/ppc/pmu_book3s_helper.c b/target/ppc/pmu_book3s_helper.c
index 43cc0eb722..58ae65e22b 100644
--- a/target/ppc/pmu_book3s_helper.c
+++ b/target/ppc/pmu_book3s_helper.c
@@ -25,6 +25,7 @@
   * and SPAPR code.
   */
  #define PPC_CPU_FREQ 10
+#define COUNTER_NEGATIVE_VAL 0x8000
  
  static uint64_t get_cycles(uint64_t icount_delta)

  {
@@ -32,22 +33,9 @@ static uint64_t get_cycles(uint64_t icount_delta)
  NANOSECONDS_PER_SECOND);
  }
  
-static void update_PMC_PM_INST_CMPL(CPUPPCState *env, int sprn,

-uint64_t icount_delta)
-{
-env->spr[sprn] += icount_delta;
-}
-
-static void update_PMC_PM_CYC(CPUPPCState *env, int sprn,
-  uint64_t icount_delta)
-{
-env->spr[sprn] += get_cycles(icount_delta);
-}
-
-static void update_programmable_PMC_reg(CPUPPCState *env, int sprn,
-uint64_t icount_delta)
+static uint8_t get_PMC_event(CPUPPCState *env, int sprn)
  {
-int event;
+int event = 0x0;
  
  switch (sprn) {

  case SPR_POWER_PMC1:
@@ -65,11 +53,35 @@ static void update_programmable_PMC_reg(CPUPPCState *env, 
int sprn,
  case SPR_POWER_PMC4:
  event = MMCR1_PMC4SEL & env->spr[SPR_POWER_MMCR1];
  break;
+case SPR_POWER_PMC5:
+event = 0x2;
+break;
+case SPR_POWER_PMC6:
+event = 0x1E;
+break;


This looks like a nice cleanup that would be better folded into an
earlier patch.


  default:
-return;
+break;
  }
  
-switch (event) {

+return event;
+}
+
+static void update_PMC_PM_INST_CMPL(CPUPPCState *env, int sprn,
+uint64_t icount_delta)
+{
+env->spr[sprn] += icount_delta;
+}
+
+static void update_PMC_PM_CYC(CPUPPCState *env, int sprn,
+  uint64_t icount_delta)
+{
+env->spr[sprn] += get_cycles(icount_delta);
+}
+
+static void update_programmable_PMC_reg(CPUPPCState *env, int sprn,
+uint64_t icount_delta)
+{
+switch (get_PMC_event(env, sprn)) {
  case 0x2:
  update_PMC_PM_INST_CMPL(env, sprn, icount_delta);
  break;
@@ -99,30 +111,57 @@ static void update_PMCs(CPUPPCState *env, uint64_t 
icount_delta)
  update_PMC_PM_CYC(env, SPR_POWER_PMC6, icount_delta);
  }
  
+static void set_PMU_excp_timer(CPUPPCState *env)

+{
+uint64_t timeout, now, remaining_val;
+
+if (!(env->spr[SPR_POWER_MMCR0] & MMCR0_PMC1CE)) {
+return;
+}
+
+remaining_val = COUNTER_NEGATIVE_VAL - env->spr[SPR_POWER_PMC1];
+
+switch (get_PMC_event(env, SPR_POWER_PMC1)) {
+case 0x2:
+timeout = icount_to_ns(remaining_val);
+break;
+case 0x1e:
+timeout = muldiv64(remaining_val, NANOSECONDS_PER_SECOND,
+   PPC_CPU_FREQ);


So.. this appears to be simulating to the guest that cycles are
occurring at a constant rate, consistent with the advertised CPU
frequency.  Which sounds right, except... it's not clear to me that
you're using the same logic to generate the values you read from the
cycles PMC (in that case it shouldn't need to reference icount at all,
right?).


'remaining_val' meaning depends on the event bein

Re: [PATCH v3 01/13] python/aqmp: Fix wait_closed work-around for python 3.6

[Niteesh G. S.]

On Thu, Aug 5, 2021 at 10:58 PM John Snow  wrote:

>
>
> On Fri, Jul 30, 2021 at 4:19 PM G S Niteesh Babu 
> wrote:
>
>> Before this patch the wait_closed work-around for python 3.6
>> fails during disconnect.
>> This is a temproray work around for which might be fixed in the
>> future or will be completely removed when the minimum python
>> version is raised to 3.7.
>>
>> This patch was originally written by John Snow 
>>
>> Signed-off-by: G S Niteesh Babu 
>> ---
>>  python/qemu/aqmp/util.py | 12 +++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/python/qemu/aqmp/util.py b/python/qemu/aqmp/util.py
>> index de0df44cbd..eaa5fc7d5f 100644
>> --- a/python/qemu/aqmp/util.py
>> +++ b/python/qemu/aqmp/util.py
>> @@ -134,7 +134,17 @@ def is_closing(writer: asyncio.StreamWriter) -> bool:
>>
>>  while not transport.is_closing():
>>  await asyncio.sleep(0)
>> -await flush(writer)
>> +
>> +# This is an ugly workaround, but it's the best I can come up with.
>> +sock = transport.get_extra_info('socket')
>> +
>> +if sock is None:
>> +# Our transport doesn't have a socket? ...
>> +# Nothing we can reasonably do.
>> +return
>> +
>> +while sock.fileno() != -1:
>> +await asyncio.sleep(0)
>>
>>
>>  def asyncio_run(coro: Coroutine[Any, Any, T], *, debug: bool = False) ->
>> T:
>> --
>> 2.17.1
>>
>>
> Sorry for the trouble. This is now included in the v3 version of my series
> and can be dropped. I hope.
>
Thanks. I'll remove this in the upcoming v4

Re: [PATCH v14] qapi: introduce 'query-x86-cpuid' QMP command.

[Eduardo Habkost]

On Tue, Aug 10, 2021 at 09:51:31AM +0300, Valeriy Vdovin wrote:
> From: Valeriy Vdovin 
> 
> Introducing new QMP command 'query-x86-cpuid'. This command can be used to
> get virtualized cpu model info generated by QEMU during VM initialization in
> the form of cpuid representation.
> 
> Diving into more details about virtual CPU generation: QEMU first parses 
> '-cpu'
> command line option. From there it takes the name of the model as the basis 
> for
> feature set of the new virtual CPU. After that it uses trailing '-cpu' 
> options,
> that state if additional cpu features should be present on the virtual CPU or
> excluded from it (tokens '+'/'-' or '=on'/'=off').
> After that QEMU checks if the host's cpu can actually support the derived
> feature set and applies host limitations to it.
> After this initialization procedure, virtual CPU has it's model and
> vendor names, and a working feature set and is ready for identification
> instructions such as CPUID.
> 
> To learn exactly how virtual CPU is presented to the guest machine via CPUID
> instruction, new QMP command can be used. By calling 'query-x86-cpuid'
> command, one can get a full listing of all CPUID leaves with subleaves which 
> are
> supported by the initialized virtual CPU.
> 
> Other than debug, the command is useful in cases when we would like to
> utilize QEMU's virtual CPU initialization routines and put the retrieved
> values into kernel CPUID overriding mechanics for more precise control
> over how various processes perceive its underlying hardware with
> container processes as a good example.
> 
> The command is specific to x86. It is currenly only implemented for KVM 
> acceleator.
> 
> Output format:
> The output is a plain list of leaf/subleaf argument combinations, that
> return 4 words in registers EAX, EBX, ECX, EDX.
>
[...]

Based on the effort being required from you to make sure this
patch is in good shape, maybe you could reconsider my suggestion
from a while ago for a single-CPUID-leaf interface, as discussed
at:
https://lore.kernel.org/qemu-devel/20210421201759.utsmhuopdmlhg...@habkost.net/

A single-CPUID-leaf qmp_query_x86_cpuid() function that is
generic and not KVM-specific can probably be implemented in ~5
lines of code.

I'm not against the interface proposed here, but you are surely
going to get more friction and more complexity to deal with.

-- 
Eduardo

Re: [PATCH 18/19] target/ppc/pmu_book3s_helper.c: add PM_CMPLU_STALL mock events

[Daniel Henrique Barboza]

On 8/10/21 1:17 AM, David Gibson wrote:

On Mon, Aug 09, 2021 at 10:10:56AM -0300, Daniel Henrique Barboza wrote:

EBB powerpc kernel test 'multi_counter_test' uses PM_CMPLU_STALL events
that we do not support. These events are related to CPU stalled/wasted
cycles while waiting for resources, cache misses and so on.

Unlike the 0xFA event added previously, there's no available equivalent
for us to use, and at this moment we can't sample those events as well.
What we can do is mock those events as if we were calculating them.

This patch implements PM_CMPLU_STALL, PM_CMPLU_STALL_FXU,
PM_CMPLU_STALL_OTHER_CMPL and PM_CMPLU_STALL_THRD mock events by giving
them a fixed amount of the total elapsed cycles.

The chosen sample values for these events (25% of total cycles for
PM_CMPLU_STALL and 5% for the other three) were chosen at random and has
no intention of being truthful with what a real PowerPC hardware would
give us. Our intention here is to make 'multi_counter_test' EBB test
pass.


Hmm.  I guess these mock values make sense for getting the kernel
tests to pass, but I'm not sure if it's a good idea in general.  Would
we be better off just reporting 0 always - that would be a strong hint
to someone attempting to analyze results that something is fishy (in
this case that they don't actually have a real CPU).


We can drop this patch and I'll add a note in the docs that the EBB kernel
test 'multi_counter_test' fails because the PMU does not implement STALL
events.


Thanks,


Daniel





Signed-off-by: Daniel Henrique Barboza 
---
  target/ppc/pmu_book3s_helper.c | 81 +-
  1 file changed, 79 insertions(+), 2 deletions(-)

diff --git a/target/ppc/pmu_book3s_helper.c b/target/ppc/pmu_book3s_helper.c
index ae7050cd62..32cf76b77f 100644
--- a/target/ppc/pmu_book3s_helper.c
+++ b/target/ppc/pmu_book3s_helper.c
@@ -92,16 +92,54 @@ static void update_PMC_PM_CYC(CPUPPCState *env, int sprn,
  env->spr[sprn] += get_cycles(icount_delta);
  }
  
+static int get_stall_ratio(uint8_t stall_event)

+{
+int stall_ratio = 0;
+
+switch (stall_event) {
+case 0xA:
+stall_ratio = 25;
+break;
+case 0x6:
+case 0x16:
+case 0x1C:
+stall_ratio = 5;
+break;
+default:
+break;
+}
+
+return stall_ratio;
+}
+
+static void update_PMC_PM_STALL(CPUPPCState *env, int sprn,
+uint64_t icount_delta,
+uint8_t stall_event)
+{
+int stall_ratio = get_stall_ratio(stall_event);
+uint64_t cycles = muldiv64(get_cycles(icount_delta), stall_ratio, 100);
+
+env->spr[sprn] += cycles;
+}
+
  static void update_programmable_PMC_reg(CPUPPCState *env, int sprn,
  uint64_t icount_delta)
  {
-switch (get_PMC_event(env, sprn)) {
+uint8_t event = get_PMC_event(env, sprn);
+
+switch (event) {
  case 0x2:
  update_PMC_PM_INST_CMPL(env, sprn, icount_delta);
  break;
  case 0x1E:
  update_PMC_PM_CYC(env, sprn, icount_delta);
  break;
+case 0xA:
+case 0x6:
+case 0x16:
+case 0x1C:
+update_PMC_PM_STALL(env, sprn, icount_delta, event);
+break;
  default:
  return;
  }
@@ -163,6 +201,34 @@ static int64_t get_CYC_timeout(CPUPPCState *env, int sprn)
  return muldiv64(remaining_cyc, NANOSECONDS_PER_SECOND, PPC_CPU_FREQ);
  }
  
+static int64_t get_stall_timeout(CPUPPCState *env, int sprn,

+ uint8_t stall_event)
+{
+uint64_t remaining_cyc;
+int stall_multiplier;
+
+if (env->spr[sprn] == 0) {
+return icount_to_ns(COUNTER_NEGATIVE_VAL);
+}
+
+if (env->spr[sprn] >= COUNTER_NEGATIVE_VAL) {
+return 0;
+}
+
+remaining_cyc = COUNTER_NEGATIVE_VAL - env->spr[sprn];
+
+/*
+ * Consider that for this stall event we'll advance the counter
+ * in a lower rate, thus requiring more cycles to overflow.
+ * E.g. for PM_CMPLU_STALL (0xA), ratio 25, it'll require
+ * 100/25 = 4 times the same amount of cycles to overflow.
+ */
+stall_multiplier = 100 / get_stall_ratio(stall_event);
+remaining_cyc *= stall_multiplier;
+
+return muldiv64(remaining_cyc, NANOSECONDS_PER_SECOND, PPC_CPU_FREQ);
+}
+
  static bool pmc_counter_negative_enabled(CPUPPCState *env, int sprn)
  {
  bool PMC14_running = !(env->spr[SPR_POWER_MMCR0] & MMCR0_FC14);
@@ -191,6 +257,7 @@ static bool pmc_counter_negative_enabled(CPUPPCState *env, 
int sprn)
  static int64_t get_counter_neg_timeout(CPUPPCState *env, int sprn)
  {
  int64_t timeout = -1;
+uint8_t event;
  
  if (!pmc_counter_negative_enabled(env, sprn)) {

  return -1;
@@ -205,13 +272,23 @@ static int64_t get_counter_neg_timeout(CPUPPCState *env, 
int sprn)
  case SPR_POWER_PMC2:
  case SPR_POWER_PMC3:
  case SPR_POWER_PMC4:
-switch (get_PMC_event(env, sprn)) {
+event = get_PMC_

Re: [PULL ppc] powernv queue

[Cédric Le Goater]

On 8/10/21 5:33 PM, Peter Maydell wrote:
> On Tue, 10 Aug 2021 at 16:32, Philippe Mathieu-Daudé  wrote:
>>
>> On 8/10/21 5:17 PM, Peter Maydell wrote:
>>> On Tue, 10 Aug 2021 at 13:46, Cédric Le Goater  wrote:
>>>>
>>>> The following changes since commit 
>>>> bccabb3a5d60182645c7749e89f21a9ff307a9eb:
>>>>
>>>>   Update version for v6.1.0-rc2 release (2021-08-04 16:56:14 +0100)
>>>>
>>>> are available in the Git repository at:
>>>>
>>>>   https://github.com/legoater/qemu/ tags/pull-powernv-20210810
>>>>
>>>> for you to fetch changes up to 91a6b62df830d51f2b6b2ea00b3c92231d0ba9dc:
>>>>
>>>>   ppc/pnv: update skiboot to commit 820d43c0a775. (2021-08-10 14:18:51 
>>>> +0200)
>>>>
>>>> 
>>>> ppc/pnv: update skiboot image
>>>>
>>>> 
>>>> Cédric Le Goater (1):
>>>>   ppc/pnv: update skiboot to commit 820d43c0a775.
>>>
>>> That skiboot changelog lists a massive set of changes, which
>>> is correspondingly a larger risk that something in there turns
>>> out to be a must-fix-for-release regression.
>>
>> There might be a misunderstanding here, per
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg828857.html
>> I understand this pull request is targetting David PPC tree,
>> not the mainstream one. David first queued this patch for 6.2,
>> and later confirmed by Cédric:
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg829160.html
> 
> Mmm. Also as Dan pointed out on irc firmware updates ought to
> be honouring the usual softfreeze rules. So I'm going to err
> on the safe side and won't be applying this.

Yes. I should have added a 'ppc-for-6.2' prefix. 

Sorry about that. 

C. 

Re: [PATCH 09/19] PPC64/TCG: Implement 'rfebb' instruction

[Daniel Henrique Barboza]

On 8/10/21 12:50 AM, David Gibson wrote:

On Mon, Aug 09, 2021 at 10:10:47AM -0300, Daniel Henrique Barboza wrote:

From: Gustavo Romero 

An Event-Based Branch (EBB) allows applications to change the NIA when a
event-based exception occurs. Event-based exceptions are enabled by
setting the Branch Event Status and Control Register (BESCR). If the
event-based exception is enabled when the exception occurs, an EBB
happens.

The EBB will:

- set the Global Enable (GE) bit of BESCR to 0;
- set bits 0-61 of the Event-Based Branch Return Register (EBBRR) to the
   effective address of the NIA that would have executed if the EBB
   didn't happen;
- Instruction fetch and execution will continue in the effective address
   contained in the Event-Based Branch Handler Register (EBBHR).

The EBB Handler will process the event and then execute the Return From
Event-Based Branch (rfebb) instruction. rfebb sets BESCR_GE and then
redirects execution to the address pointed in EBBRR. This process is
described in the PowerISA v3.1, Book II, Chapter 6 [1].

This patch implements the rfebb instruction. Descriptions of all
relevant BESCR bits are also added - this patch is only using BESCR_GE,
but next patches will use the remaining bits.

Note that we're implementing the extended rfebb mnemonic (BESCR_GE is
being always set to 1). The basic rfebb instruction would accept an
operand that would be used to set GE.

[1] https://wiki.raptorcs.com/w/images/f/f5/PowerISA_public.v3.1.pdf

CC: Gustavo Romero 
Signed-off-by: Gustavo Romero 
Signed-off-by: Daniel Henrique Barboza 
---
  target/ppc/cpu.h   | 12 
  target/ppc/translate.c | 21 +
  2 files changed, 33 insertions(+)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index afd9cd402b..ae431e65be 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -358,6 +358,18 @@ typedef struct ppc_v3_pate_t {
  #define MMCR1_PMC3SEL PPC_BITMASK(48, 55)
  #define MMCR1_PMC4SEL PPC_BITMASK(56, 63)
  
+/* EBB/BESCR bits */

+/* Global Enable */
+#define BESCR_GE PPC_BIT(0)
+/* External Event-based Exception Enable */
+#define BESCR_EE PPC_BIT(30)
+/* Performance Monitor Event-based Exception Enable */
+#define BESCR_PME PPC_BIT(31)
+/* External Event-based Exception Occurred */
+#define BESCR_EEO PPC_BIT(62)
+/* Performance Monitor Event-based Exception Occurred */
+#define BESCR_PMEO PPC_BIT(63)
+
  /* LPCR bits */
  #define LPCR_VPM0 PPC_BIT(0)
  #define LPCR_VPM1 PPC_BIT(1)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 62356cfadf..afc254a03f 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -2701,6 +2701,26 @@ static void gen_darn(DisasContext *ctx)
  }
  }
  }
+
+/* rfebb */
+static void gen_rfebb(DisasContext *ctx)


Oof.. not necessarily a nack, but it would be nice to implement any
new instructions using the disastree path rather than the old ppc
specific decode logic.



I'm not sure what is the disastree path. Is it similar to how rfscv is
implemented?




Daniel







+{
+TCGv target = tcg_temp_new();
+TCGv bescr = tcg_temp_new();
+
+gen_load_spr(target, SPR_EBBRR);
+tcg_gen_mov_tl(cpu_nip, target);
+
+gen_load_spr(bescr, SPR_BESCR);
+tcg_gen_ori_tl(bescr, bescr, BESCR_GE);
+gen_store_spr(SPR_BESCR, bescr);
+
+ctx->base.is_jmp = DISAS_EXIT;
+
+tcg_temp_free(target);
+tcg_temp_free(bescr);
+}
+
  #endif
  
  /*** Integer rotate***/

@@ -7724,6 +7744,7 @@ GEN_HANDLER(popcntd, 0x1F, 0x1A, 0x0F, 0xF801, 
PPC_POPCNTWD),
  GEN_HANDLER(cntlzd, 0x1F, 0x1A, 0x01, 0x, PPC_64B),
  GEN_HANDLER_E(cnttzd, 0x1F, 0x1A, 0x11, 0x, PPC_NONE, PPC2_ISA300),
  GEN_HANDLER_E(darn, 0x1F, 0x13, 0x17, 0x001CF801, PPC_NONE, PPC2_ISA300),
+GEN_HANDLER_E(rfebb, 0x13, 0x12, 0x04, 0x03FFF001, PPC_NONE, PPC2_ISA207S),
  GEN_HANDLER_E(prtyd, 0x1F, 0x1A, 0x05, 0xF801, PPC_NONE, PPC2_ISA205),
  GEN_HANDLER_E(bpermd, 0x1F, 0x1C, 0x07, 0x0001, PPC_NONE, 
PPC2_PERM_ISA206),
  #endif

Re: [PATCH-for-6.2] tests/acceptance: Allow overwrite smp and memory

[Philippe Mathieu-Daudé]

Subject: "Allow overwrite default smp and memsize command line options"?

On 8/10/21 9:18 PM, Philippe Mathieu-Daudé wrote:
> On 8/3/21 12:22 AM, Ahmed Abouzied wrote:
>> Removes the hard-coded values in setUp(). Class inheriting from
>> avocado_qemu.LinuxTest can overwrite the default smp and memory instead.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/453
>> Signed-off-by: Ahmed Abouzied 
>> ---
>>  tests/acceptance/avocado_qemu/__init__.py | 6 --
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/acceptance/avocado_qemu/__init__.py 
>> b/tests/acceptance/avocado_qemu/__init__.py
>> index 2c4fef3e14..2639b89c84 100644
>> --- a/tests/acceptance/avocado_qemu/__init__.py
>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>> @@ -441,6 +441,8 @@ class LinuxTest(Test, LinuxSSHMixIn):
>>  distro = None
>>  username = 'root'
>>  password = 'password'
>> +smp = '2'
>> +memory = '1024'
>>  
>>  def _set_distro(self):
>>  distro_name = self.params.get(
>> @@ -471,8 +473,8 @@ def _set_distro(self):
>>  def setUp(self, ssh_pubkey=None, network_device_type='virtio-net'):
>>  super(LinuxTest, self).setUp()
>>  self._set_distro()
>> -self.vm.add_args('-smp', '2')
> 
> I don't understand why we use 2 as default value, but this is unrelated
> to this patch, so:
> Reviewed-by: Philippe Mathieu-Daudé 
> 
>> -self.vm.add_args('-m', '1024')
>> +self.vm.add_args('-smp', self.smp)
>> +self.vm.add_args('-m', self.memory)
>>  # The following network device allows for SSH connections
>>  self.vm.add_args('-netdev', 'user,id=vnet,hostfwd=:127.0.0.1:0-:22',
>>   '-device', '%s,netdev=vnet' % network_device_type)
>>
> 

Re: [PATCH qemu 1/1] docs: how to use gdb with unix sockets

[Philippe Mathieu-Daudé]

Yay! Posting from sr.ht works \o/

On 8/10/21 6:04 PM, ~archi42 wrote:
> From: Sebastian Meyer 
> 
> Signed-off-by: Sebastian Meyer 
> ---
>  docs/system/gdb.rst | 24 +++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/system/gdb.rst b/docs/system/gdb.rst
> index 144d083df3..2ff4d6aab5 100644
> --- a/docs/system/gdb.rst
> +++ b/docs/system/gdb.rst
> @@ -15,7 +15,8 @@ The ``-s`` option will make QEMU listen for an incoming 
> connection
>  from gdb on TCP port 1234, and ``-S`` will make QEMU not start the
>  guest until you tell it to from gdb. (If you want to specify which
>  TCP port to use or to use something other than TCP for the gdbstub
> -connection, use the ``-gdb dev`` option instead of ``-s``.)
> +connection, use the ``-gdb dev`` option instead of ``-s``. See
> +further below for an example using unix sockets.)

Could we insert a link to the 'Using unix sockets' chapter?

>  
>  .. parsed-literal::
>  
> @@ -168,3 +169,24 @@ The memory mode can be checked by sending the following 
> command:
>  
>  ``maintenance packet Qqemu.PhyMemMode:0``
>  This will change it back to normal memory mode.
> +
> +Using unix sockets
> +^^
> +
> +An alternate method for connecting gdb to the qemu gdbstub are unix

"QEMU"

> +sockets (if supported by your operating system). This is useful when
> +running serveral tests in parallel and/or you do not know a free tcp

"several", "know" -> "have", "TCP"

> +port a priori (e.g. when running automated tests).
> +First create a new chardev with the appropriate options, then

Drop "new"?

> +instruct the gdbserver to use that device::
> +
> +.. parsed-literal::
> +
> +   |qemu_system| -chardev 
> socket,path=/tmp/gdb-socket,server=on,wait=off,id=gdb0 -gdb chardev:gdb0 -S 
> -kernel bzImage -hda rootdisk.img -append "root=/dev/hda"

Maybe simply "-chardev
socket,path=/tmp/gdb-socket,server=on,wait=off,id=gdb0 -gdb chardev:gdb0
..."?

> +
> +Start gdb as before, but this time connect using the path to
> +the socket::
> +
> +   (gdb) target remote /tmp/gdb-socket
> +
> +Please mind that this usually requires gdb version 9.0 or newer.

"Note gdb version 9.0 or newer is required."?

Re: [PATCH-for-6.2] tests/acceptance: Allow overwrite smp and memory

[Philippe Mathieu-Daudé]

On 8/3/21 12:22 AM, Ahmed Abouzied wrote:
> Removes the hard-coded values in setUp(). Class inheriting from
> avocado_qemu.LinuxTest can overwrite the default smp and memory instead.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/453
> Signed-off-by: Ahmed Abouzied 
> ---
>  tests/acceptance/avocado_qemu/__init__.py | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/acceptance/avocado_qemu/__init__.py 
> b/tests/acceptance/avocado_qemu/__init__.py
> index 2c4fef3e14..2639b89c84 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -441,6 +441,8 @@ class LinuxTest(Test, LinuxSSHMixIn):
>  distro = None
>  username = 'root'
>  password = 'password'
> +smp = '2'
> +memory = '1024'
>  
>  def _set_distro(self):
>  distro_name = self.params.get(
> @@ -471,8 +473,8 @@ def _set_distro(self):
>  def setUp(self, ssh_pubkey=None, network_device_type='virtio-net'):
>  super(LinuxTest, self).setUp()
>  self._set_distro()
> -self.vm.add_args('-smp', '2')

I don't understand why we use 2 as default value, but this is unrelated
to this patch, so:
Reviewed-by: Philippe Mathieu-Daudé 

> -self.vm.add_args('-m', '1024')
> +self.vm.add_args('-smp', self.smp)
> +self.vm.add_args('-m', self.memory)
>  # The following network device allows for SSH connections
>  self.vm.add_args('-netdev', 'user,id=vnet,hostfwd=:127.0.0.1:0-:22',
>   '-device', '%s,netdev=vnet' % network_device_type)
> 

Re: [PATCH v14] qapi: introduce 'query-x86-cpuid' QMP command.

[Philippe Mathieu-Daudé]

On 8/10/21 8:51 AM, Valeriy Vdovin wrote:
> From: Valeriy Vdovin 
> 
> Introducing new QMP command 'query-x86-cpuid'. This command can be used to
> get virtualized cpu model info generated by QEMU during VM initialization in
> the form of cpuid representation.
> 
> Diving into more details about virtual CPU generation: QEMU first parses 
> '-cpu'
> command line option. From there it takes the name of the model as the basis 
> for
> feature set of the new virtual CPU. After that it uses trailing '-cpu' 
> options,
> that state if additional cpu features should be present on the virtual CPU or
> excluded from it (tokens '+'/'-' or '=on'/'=off').
> After that QEMU checks if the host's cpu can actually support the derived
> feature set and applies host limitations to it.
> After this initialization procedure, virtual CPU has it's model and
> vendor names, and a working feature set and is ready for identification
> instructions such as CPUID.
> 
> To learn exactly how virtual CPU is presented to the guest machine via CPUID
> instruction, new QMP command can be used. By calling 'query-x86-cpuid'
> command, one can get a full listing of all CPUID leaves with subleaves which 
> are
> supported by the initialized virtual CPU.
> 
> Other than debug, the command is useful in cases when we would like to
> utilize QEMU's virtual CPU initialization routines and put the retrieved
> values into kernel CPUID overriding mechanics for more precise control
> over how various processes perceive its underlying hardware with
> container processes as a good example.
> 
> The command is specific to x86. It is currenly only implemented for KVM 
> acceleator.
> 
> Output format:
> The output is a plain list of leaf/subleaf argument combinations, that
> return 4 words in registers EAX, EBX, ECX, EDX.
> 
> Use example:
> qmp_request: {
>   "execute": "query-x86-cpuid"
> }
> 
> qmp_response: {
>   "return": [
> {
>   "eax": 1073741825,
>   "edx": 77,
>   "in-eax": 1073741824,
>   "ecx": 1447775574,
>   "ebx": 1263359563
> },
> {
>   "eax": 16777339,
>   "edx": 0,
>   "in-eax": 1073741825,
>   "ecx": 0,
>   "ebx": 0
> },
> {
>   "eax": 13,
>   "edx": 1231384169,
>   "in-eax": 0,
>   "ecx": 1818588270,
>   "ebx": 1970169159
> },
> {
>   "eax": 198354,
>   "edx": 126614527,
>   "in-eax": 1,
>   "ecx": 2176328193,
>   "ebx": 2048
> },
> 
> {
>   "eax": 12328,
>   "edx": 0,
>   "in-eax": 2147483656,
>   "ecx": 0,
>   "ebx": 0
> }
>   ]
> }
> 
> Signed-off-by: Valeriy Vdovin 
> ---
> v2: - Removed leaf/subleaf iterators.
> - Modified cpu_x86_cpuid to return false in cases when count is
>   greater than supported subleaves.
> v3: - Fixed structure name coding style.
> - Added more comments
> - Ensured buildability for non-x86 targets.
> v4: - Fixed cpu_x86_cpuid return value logic and handling of 0xA leaf.
> - Fixed comments.
> - Removed target check in qmp_query_cpu_model_cpuid.
> v5: - Added error handling code in qmp_query_cpu_model_cpuid
> v6: - Fixed error handling code. Added method to query_error_class
> v7: - Changed implementation in favor of cached cpuid_data for
>   KVM_SET_CPUID2
> v8: - Renamed qmp method to query-kvm-cpuid and some fields in response.
> - Modified documentation to qmp method
> - Removed helper struct declaration
> v9: - Renamed 'in_eax' / 'in_ecx' fields to 'in-eax' / 'in-ecx'
> - Pasted more complete response to commit message.
> v10:
> - Subject changed
> - Fixes in commit message
> - Small fixes in QMP command docs
> v11:
> - Added explanation about CONFIG_KVM to the commit message.
> v12:
> - Changed title from query-kvm-cpuid to query-x86-cpuid
> - Removed CONFIG_KVM ifdefs
> - Added detailed error messages for some stub/unimplemented cases.
> v13:
> - Tagged with since 6.2
> v14:
> - Rebased to latest master 632eda54043d6f26ff87dac16233e14b4708b967
> - Added note about error return cases in api documentation.
> 
>  qapi/machine-target.json   | 46 ++
>  softmmu/cpus.c |  2 +-
>  target/i386/kvm/kvm-stub.c | 10 
>  target/i386/kvm/kvm.c  | 51 ++
>  tests/qtest/qmp-cmd-test.c |  1 +
>  5 files changed, 109 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index e7811654b7..71648a4f56 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -329,3 +329,49 @@
>  ##
>  { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
>'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) 
> || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
> +
> +##
> +# @CpuidEntry:
> +#
> +# A single entry of a CPUID response.
> +#
> +# One entry holds full set of information (leaf) returned to the guest
> +# i

Re: [PATCH v12] qapi: introduce 'query-x86-cpuid' QMP command.

[Eduardo Habkost]

On Sat, Aug 07, 2021 at 04:22:42PM +0200, Markus Armbruster wrote:
> Is this intended to be a stable interface?  Interfaces intended just for
> debugging usually aren't.

I don't think we need to make it a stable interface, but I won't
mind if we declare it stable.

> 
> Eduardo, what's your take on this version?
> 

I sent my comments as reply to v14:
https://lore.kernel.org/qemu-devel/20210810185245.kivvmrmvew6e5...@habkost.net/

-- 
Eduardo

Re: [PATCH v14] qapi: introduce 'query-x86-cpuid' QMP command.

[Eduardo Habkost]

On Tue, Aug 10, 2021 at 09:51:31AM +0300, Valeriy Vdovin wrote:
> From: Valeriy Vdovin 
> 
> Introducing new QMP command 'query-x86-cpuid'. This command can be used to
> get virtualized cpu model info generated by QEMU during VM initialization in
> the form of cpuid representation.
> 
> Diving into more details about virtual CPU generation: QEMU first parses 
> '-cpu'
> command line option. From there it takes the name of the model as the basis 
> for
> feature set of the new virtual CPU. After that it uses trailing '-cpu' 
> options,
> that state if additional cpu features should be present on the virtual CPU or
> excluded from it (tokens '+'/'-' or '=on'/'=off').
> After that QEMU checks if the host's cpu can actually support the derived
> feature set and applies host limitations to it.
> After this initialization procedure, virtual CPU has it's model and
> vendor names, and a working feature set and is ready for identification
> instructions such as CPUID.
> 
> To learn exactly how virtual CPU is presented to the guest machine via CPUID
> instruction, new QMP command can be used. By calling 'query-x86-cpuid'
> command, one can get a full listing of all CPUID leaves with subleaves which 
> are
> supported by the initialized virtual CPU.
> 
> Other than debug, the command is useful in cases when we would like to
> utilize QEMU's virtual CPU initialization routines and put the retrieved
> values into kernel CPUID overriding mechanics for more precise control
> over how various processes perceive its underlying hardware with
> container processes as a good example.
> 
> The command is specific to x86. It is currenly only implemented for KVM 
> acceleator.
> 
> Output format:
> The output is a plain list of leaf/subleaf argument combinations, that
> return 4 words in registers EAX, EBX, ECX, EDX.
> 
> Use example:
> qmp_request: {
>   "execute": "query-x86-cpuid"
> }
> 
> qmp_response: {
>   "return": [
> {
>   "eax": 1073741825,
>   "edx": 77,
>   "in-eax": 1073741824,
>   "ecx": 1447775574,
>   "ebx": 1263359563
> },
> {
>   "eax": 16777339,
>   "edx": 0,
>   "in-eax": 1073741825,
>   "ecx": 0,
>   "ebx": 0
> },
> {
>   "eax": 13,
>   "edx": 1231384169,
>   "in-eax": 0,
>   "ecx": 1818588270,
>   "ebx": 1970169159
> },
> {
>   "eax": 198354,
>   "edx": 126614527,
>   "in-eax": 1,
>   "ecx": 2176328193,
>   "ebx": 2048
> },
> 
> {
>   "eax": 12328,
>   "edx": 0,
>   "in-eax": 2147483656,
>   "ecx": 0,
>   "ebx": 0
> }
>   ]
> }
> 
> Signed-off-by: Valeriy Vdovin 
> ---
> v2: - Removed leaf/subleaf iterators.
> - Modified cpu_x86_cpuid to return false in cases when count is
>   greater than supported subleaves.
> v3: - Fixed structure name coding style.
> - Added more comments
> - Ensured buildability for non-x86 targets.
> v4: - Fixed cpu_x86_cpuid return value logic and handling of 0xA leaf.
> - Fixed comments.
> - Removed target check in qmp_query_cpu_model_cpuid.
> v5: - Added error handling code in qmp_query_cpu_model_cpuid
> v6: - Fixed error handling code. Added method to query_error_class
> v7: - Changed implementation in favor of cached cpuid_data for
>   KVM_SET_CPUID2
> v8: - Renamed qmp method to query-kvm-cpuid and some fields in response.
> - Modified documentation to qmp method
> - Removed helper struct declaration
> v9: - Renamed 'in_eax' / 'in_ecx' fields to 'in-eax' / 'in-ecx'
> - Pasted more complete response to commit message.
> v10:
> - Subject changed
> - Fixes in commit message
> - Small fixes in QMP command docs
> v11:
> - Added explanation about CONFIG_KVM to the commit message.
> v12:
> - Changed title from query-kvm-cpuid to query-x86-cpuid
> - Removed CONFIG_KVM ifdefs
> - Added detailed error messages for some stub/unimplemented cases.
> v13:
> - Tagged with since 6.2
> v14:
> - Rebased to latest master 632eda54043d6f26ff87dac16233e14b4708b967
> - Added note about error return cases in api documentation.
> 
>  qapi/machine-target.json   | 46 ++
>  softmmu/cpus.c |  2 +-
>  target/i386/kvm/kvm-stub.c | 10 
>  target/i386/kvm/kvm.c  | 51 ++
>  tests/qtest/qmp-cmd-test.c |  1 +
>  5 files changed, 109 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index e7811654b7..71648a4f56 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -329,3 +329,49 @@
>  ##
>  { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
>'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) 
> || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
> +
> +##
> +# @CpuidEntry:
> +#
> +# A single entry of a CPUID response.
> +#
> +# One entry holds full set of information (leaf) return

Re: [PATCH for 6.2 16/49] bsd-user: elfload: simplify bswap a bit.

[Richard Henderson]

On 8/7/21 11:42 AM, Warner Losh wrote:

@@ -367,9 +383,7 @@ static abi_ulong load_elf_interp(struct elfhdr 
*interp_elf_ex,
  last_bss = 0;
  error = 0;
  
-#ifdef BSWAP_NEEDED

  bswap_ehdr(interp_elf_ex);
-#endif
  /* First of all, some simple consistency checks */
  if ((interp_elf_ex->e_type != ET_EXEC &&
   interp_elf_ex->e_type != ET_DYN) ||


Existing bug here: You should be checking, at minimum, EI_CLASS and EI_DATA before bswap. 
 Otherwise you don't even know you're swapping the correct structure.


I noticed this much later in the series.

r~

Re: [PATCH v2] fsl-imx6ul: Instantiate SAI1/2/3 and ASRC as unimplemented devices

[Guenter Roeck]

On 8/10/21 9:34 AM, Philippe Mathieu-Daudé wrote:

On 8/10/21 6:03 PM, Guenter Roeck wrote:

Instantiate SAI1/2/3 and ASRC as unimplemented devices to avoid random
Linux kernel crashes, such as

Unhandled fault: external abort on non-linefetch (0x808) at 0xd1580010^M
pgd = (ptrval)^M
[d1580010] *pgd=8231b811, *pte=02034653, *ppte=02034453^M
Internal error: : 808 [#1] SMP ARM^M
...
[] (regmap_mmio_write32le) from [] 
(regmap_mmio_write+0x3c/0x54)^M
[] (regmap_mmio_write) from [] (_regmap_write+0x4c/0x1f0)^M
[] (_regmap_write) from [] (_regmap_update_bits+0xe4/0xec)^M
[] (_regmap_update_bits) from [] 
(regmap_update_bits_base+0x50/0x74)^M
[] (regmap_update_bits_base) from [] 
(fsl_asrc_runtime_resume+0x1e4/0x21c)^M
[] (fsl_asrc_runtime_resume) from [] 
(__rpm_callback+0x3c/0x108)^M
[] (__rpm_callback) from [] (rpm_callback+0x60/0x64)^M
[] (rpm_callback) from [] (rpm_resume+0x5cc/0x808)^M
[] (rpm_resume) from [] (__pm_runtime_resume+0x60/0xa0)^M
[] (__pm_runtime_resume) from [] 
(fsl_asrc_probe+0x2a8/0x708)^M
[] (fsl_asrc_probe) from [] (platform_probe+0x58/0xb8)^M
[] (platform_probe) from [] 
(really_probe.part.0+0x9c/0x334)^M
[] (really_probe.part.0) from [] 
(__driver_probe_device+0xa0/0x138)^M
[] (__driver_probe_device) from [] 
(driver_probe_device+0x30/0xc8)^M
[] (driver_probe_device) from [] 
(__driver_attach+0x90/0x130)^M
[] (__driver_attach) from [] (bus_for_each_dev+0x78/0xb8)^M
[] (bus_for_each_dev) from [] (bus_add_driver+0xf0/0x1d8)^M
[] (bus_add_driver) from [] (driver_register+0x88/0x118)^M
[] (driver_register) from [] (do_one_initcall+0x7c/0x3a4)^M
[] (do_one_initcall) from [] 
(kernel_init_freeable+0x198/0x22c)^M
[] (kernel_init_freeable) from [] (kernel_init+0x10/0x128)^M
[] (kernel_init) from [] (ret_from_fork+0x14/0x38)^M

or

Unhandled fault: external abort on non-linefetch (0x808) at 0xd19b^M
pgd = (ptrval)^M
[d19b] *pgd=82711811, *pte=308a0653, *ppte=308a0453^M
Internal error: : 808 [#1] SMP ARM^M
...
[] (regmap_mmio_write32le) from [] 
(regmap_mmio_write+0x3c/0x54)^M
[] (regmap_mmio_write) from [] (_regmap_write+0x4c/0x1f0)^M
[] (_regmap_write) from [] (regmap_write+0x3c/0x60)^M
[] (regmap_write) from [] 
(fsl_sai_runtime_resume+0x9c/0x1ec)^M
[] (fsl_sai_runtime_resume) from [] 
(__rpm_callback+0x3c/0x108)^M
[] (__rpm_callback) from [] (rpm_callback+0x60/0x64)^M
[] (rpm_callback) from [] (rpm_resume+0x5cc/0x808)^M
[] (rpm_resume) from [] (__pm_runtime_resume+0x60/0xa0)^M
[] (__pm_runtime_resume) from [] 
(fsl_sai_probe+0x2b8/0x65c)^M
[] (fsl_sai_probe) from [] (platform_probe+0x58/0xb8)^M
[] (platform_probe) from [] 
(really_probe.part.0+0x9c/0x334)^M
[] (really_probe.part.0) from [] 
(__driver_probe_device+0xa0/0x138)^M
[] (__driver_probe_device) from [] 
(driver_probe_device+0x30/0xc8)^M
[] (driver_probe_device) from [] 
(__driver_attach+0x90/0x130)^M
[] (__driver_attach) from [] (bus_for_each_dev+0x78/0xb8)^M
[] (bus_for_each_dev) from [] (bus_add_driver+0xf0/0x1d8)^M
[] (bus_add_driver) from [] (driver_register+0x88/0x118)^M
[] (driver_register) from [] (do_one_initcall+0x7c/0x3a4)^M
[] (do_one_initcall) from [] 
(kernel_init_freeable+0x198/0x22c)^M
[] (kernel_init_freeable) from [] (kernel_init+0x10/0x128)^M
[] (kernel_init) from [] (ret_from_fork+0x14/0x38)^M

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Guenter Roeck 
---
v2: Updated description to include tracebacks and to use the term 'instantiate'
 Added inline comments describing devices in more detail


Thank you!

Peter you might want to s/^M// when queueing ;)



Outch. Sorry for that.

Guenter


 Added Philippe's Reviewed-by: tag

  hw/arm/fsl-imx6ul.c | 12 
  1 file changed, 12 insertions(+)

diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
index e0128d7316..1d1a708dd9 100644
--- a/hw/arm/fsl-imx6ul.c
+++ b/hw/arm/fsl-imx6ul.c
@@ -534,6 +534,13 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error 
**errp)
   */
  create_unimplemented_device("sdma", FSL_IMX6UL_SDMA_ADDR, 0x4000);
  
+/*

+ * SAI (Audio SSI (Synchronous Serial Interface))
+ */
+create_unimplemented_device("sai1", FSL_IMX6UL_SAI1_ADDR, 0x4000);
+create_unimplemented_device("sai2", FSL_IMX6UL_SAI2_ADDR, 0x4000);
+create_unimplemented_device("sai3", FSL_IMX6UL_SAI3_ADDR, 0x4000);
+
  /*
   * PWM
   */
@@ -542,6 +549,11 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error 
**errp)
  create_unimplemented_device("pwm3", FSL_IMX6UL_PWM3_ADDR, 0x4000);
  create_unimplemented_device("pwm4", FSL_IMX6UL_PWM4_ADDR, 0x4000);
  
+/*

+ * Audio ASRC (asynchronous sample rate converter)
+ */
+create_unimplemented_device("asrc", FSL_IMX6UL_ASRC_ADDR, 0x4000);
+
  /*
   * CAN
   */

Re: [CXL volatile MEM] - Qemu command to turn on HMAT and NUMA fails with assertion

[Ben Widawsky]

Thanks Dave.

Samarth,

Easiest is to just use our run_qemu and figure out the diffs (--cmdline will
print the qemu commandline):
https://github.com/pmem/run_qemu

If you're not able to figure it out after that, please let me know.

On 21-08-10 17:38:16, Samarth Saxena wrote:
> Thanks Dave,
> 
> The Qemu version is qemu-6.0.50.
> 
> I am trying to capture the stack and will place it ASAP.
> 
> Regards,
> Samarth
> 
> 
> -Original Message-
> From: Dr. David Alan Gilbert  
> Sent: Tuesday, August 10, 2021 4:58 PM
> To: Samarth Saxena ; ben.widaw...@intel.com
> Cc: qemu-devel@nongnu.org
> Subject: Re: [CXL volatile MEM] - Qemu command to turn on HMAT and NUMA fails 
> with assertion
> 
> EXTERNAL MAIL
> 
> 
> * Samarth Saxena (samar...@cadence.com) wrote:
> > Hi All,
> > 
> > I am trying the following command to run Qemu with Kernel 5.14.
> 
> cc'ing in Ben who I think owns the CXL stuff.
> 
> > qemu-system-x86_64 -M q35,accel=kvm,nvdimm=on,cxl=on,hmat=on -m 
> > 8448M,slots=2,maxmem=16G -smp 8,sockets=2,cores=2,threads=2 -hda 
> > /lan/dscratch/singhabh/shradha/ubuntu-20.10-64_with_orig_driver_backup
> > .qcow2 -enable-kvm -usbdevice tablet -L $VB_ROOT/etc/vm/common/ 
> > -object memory-backend-ram,id=cxl-ram,share=on,size=256M -device 
> > "pxb-cxl,id=cxl.0,bus=pcie.0,bus_nr=52,uid=0,len-window-base=1,window-
> > base[0]=0x4c000,memdev[0]=cxl-ram" -device 
> > cxl-rp,id=rp0,bus=cxl.0,addr=0.0,chassis=0,slot=0 -device 
> > cxl-type3,bus=rp0,memdev=cxl-ram,id=cxl-vmem0,size=256M -numa 
> > node,nodeid=0,memdev=cxl-ram -object 
> > memory-backend-ram,id=other-ram,size=8G,prealloc=n,share=off -numa 
> > node,nodeid=1,memdev=other-ram,initiator=0 -numa 
> > cpu,node-id=0,socket-id=0 -numa cpu,node-id=0,socket-id=1
> 
> You probably need to state which qemu tree and version you're using here.
> 
> > I get the following crash before the machine starts to boot.
> > 
> > qemu-system-x86_64: ../softmmu/memory.c:2443: 
> > memory_region_add_subregion_common: Assertion `!subregion->container' 
> > failed.
> 
> It's probably best to check with Ben, but you'll want a backtrace and figure 
> out which subregion and region you're dealing with.
> 
> Dave
> 
> > 
> > Please help me find what's wrong here.
> > 
> > Regards,
> > [CadenceLogoRed185Regcopy1583174817new51584636989.png] > ence.com/en_US/home.html>
> > Samarth Saxena
> > Sr Principal Software Engineer
> > T: +911204308300
> > [UIcorrectsize1583179003.png]
> > [16066EmailSignatureFortune100Best2021White92x1271617625037.png] > ://www.cadence.com/en_US/home/company/careers.html>
> > 
> > 
> > 
> > 
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> 

Re: [PATCH for 6.2 49/49] bsd-user: Add '-0 argv0' option to bsd-user/main.c

[Richard Henderson]

On 8/7/21 11:42 AM, Warner Losh wrote:

From: Colin Percival

Previously it was impossible to emulate a program with a file name
different from its argv[0].  With this change, you can run
 qemu -0 fakename realname args
which runs the program "realname" with an argv of "fakename args".

Signed-off-by: Colin Percival
Signed-off-by: Warner Losh
---
  bsd-user/main.c | 5 +
  1 file changed, 5 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for 6.2 48/49] bsd-user: Implement cpu_copy() helper routine

[Richard Henderson]

On 8/7/21 11:42 AM, Warner Losh wrote:

From: Warner Losh

cpu_copy shouldbe called when processes are creating new threads. It
copies the current state of the CPU to a new cpu state needed for the
new thread.

Signed-off-by: Stacey Son
Signed-off-by: Warner Losh
Signed-off-by: Justin Hibbits
---
  bsd-user/main.c | 30 ++
  1 file changed, 30 insertions(+)


Reviewed-by: Richard Henderson 


r~

Re: [PATCH for 6.2 45/49] bsd-user: Make guest_base an unsigned long

[Warner Losh]

On Tue, Aug 10, 2021 at 11:58 AM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 8/7/21 11:42 AM, Warner Losh wrote:
> > From: Warner Losh 
> >
> > Make the guest_base a plan, uneigned long rather than a uintptr_t. It
> > isn't really a pointer.
>
> NACK: 5ca870b9f6c.  It most certainly is a host pointer.
>

OK. I'll drop this one and redo in our other branch. Shouldn't be hard.

Re: [PATCH for 6.2 47/49] bsd-user: Implement interlock for atomic operations

[Richard Henderson]

On 8/7/21 11:42 AM, Warner Losh wrote:

From: Warner Losh

Implement the internlock in fork_start() and fork_end() to properly cope
with atomic operations and to safely keep state for parent and child
processes.

Signed-off-by: Stacey Son
Signed-off-by: Warner Losh
---
  bsd-user/main.c | 23 +++
  1 file changed, 23 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for 6.2 46/49] bsd-user: move qemu_log to later in the file

[Richard Henderson]

On 8/7/21 11:42 AM, Warner Losh wrote:

From: Warner Losh

Signed-off-by: Warner Losh
---
  bsd-user/main.c | 17 +
  1 file changed, 9 insertions(+), 8 deletions(-)


Acked-by: Richard Henderson 

r~

Re: [PATCH for 6.2 45/49] bsd-user: Make guest_base an unsigned long

[Richard Henderson]

On 8/7/21 11:42 AM, Warner Losh wrote:

From: Warner Losh 

Make the guest_base a plan, uneigned long rather than a uintptr_t. It
isn't really a pointer.


NACK: 5ca870b9f6c.  It most certainly is a host pointer.


r~

Re: [PATCH] vl: fix machine option containing underscores

[Paolo Bonzini]

On 10/08/21 15:12, Jean-Philippe Brucker wrote:

My first take was renaming default_bus_bypass_iommu, since it's the only
machine option with underscores, 


We should do that, since the underscore variant still works and the 
result is a simple one-line patch.


Paolo


but then we'd want to rename
bypass_iommu as well for consistency and update all the docs. I prefer
this but don't mind either way.

[PATCH] fsl-imx7: Instantiate SAI1/2/3 as unimplemented devices

[Guenter Roeck]

Instantiate SAI1/2/3 as unimplemented devices to avoid Linux kernel crashes
such as the following.

Unhandled fault: external abort on non-linefetch (0x808) at 0xd19b
pgd = (ptrval)
[d19b] *pgd=82711811, *pte=308a0653, *ppte=308a0453
Internal error: : 808 [#1] SMP ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc5 #1
...
[] (regmap_mmio_write32le) from [] 
(regmap_mmio_write+0x3c/0x54)
[] (regmap_mmio_write) from [] (_regmap_write+0x4c/0x1f0)
[] (_regmap_write) from [] (regmap_write+0x3c/0x60)
[] (regmap_write) from [] 
(fsl_sai_runtime_resume+0x9c/0x1ec)
[] (fsl_sai_runtime_resume) from [] 
(__rpm_callback+0x3c/0x108)
[] (__rpm_callback) from [] (rpm_callback+0x60/0x64)
[] (rpm_callback) from [] (rpm_resume+0x5cc/0x808)
[] (rpm_resume) from [] (__pm_runtime_resume+0x60/0xa0)
[] (__pm_runtime_resume) from [] (fsl_sai_probe+0x2b8/0x65c)
[] (fsl_sai_probe) from [] (platform_probe+0x58/0xb8)
[] (platform_probe) from [] (really_probe.part.0+0x9c/0x334)
[] (really_probe.part.0) from [] 
(__driver_probe_device+0xa0/0x138)
[] (__driver_probe_device) from [] 
(driver_probe_device+0x30/0xc8)
[] (driver_probe_device) from [] 
(__driver_attach+0x90/0x130)
[] (__driver_attach) from [] (bus_for_each_dev+0x78/0xb8)
[] (bus_for_each_dev) from [] (bus_add_driver+0xf0/0x1d8)
[] (bus_add_driver) from [] (driver_register+0x88/0x118)
[] (driver_register) from [] (do_one_initcall+0x7c/0x3a4)
[] (do_one_initcall) from [] 
(kernel_init_freeable+0x198/0x22c)
[] (kernel_init_freeable) from [] (kernel_init+0x10/0x128)
[] (kernel_init) from [] (ret_from_fork+0x14/0x38)

Signed-off-by: Guenter Roeck 
---
 hw/arm/fsl-imx7.c | 7 +++
 include/hw/arm/fsl-imx7.h | 5 +
 2 files changed, 12 insertions(+)

diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
index 908aa6ca8a..81daac7747 100644
--- a/hw/arm/fsl-imx7.c
+++ b/hw/arm/fsl-imx7.c
@@ -483,6 +483,13 @@ static void fsl_imx7_realize(DeviceState *dev, Error 
**errp)
 create_unimplemented_device("can1", FSL_IMX7_CAN1_ADDR, 
FSL_IMX7_CANn_SIZE);
 create_unimplemented_device("can2", FSL_IMX7_CAN2_ADDR, 
FSL_IMX7_CANn_SIZE);
 
+/*
+ * SAI (Audio SSI (Synchronous Serial Interface))
+ */
+create_unimplemented_device("sai1", FSL_IMX7_SAI1_ADDR, 
FSL_IMX7_SAIn_SIZE);
+create_unimplemented_device("sai2", FSL_IMX7_SAI2_ADDR, 
FSL_IMX7_SAIn_SIZE);
+create_unimplemented_device("sai2", FSL_IMX7_SAI3_ADDR, 
FSL_IMX7_SAIn_SIZE);
+
 /*
  * OCOTP
  */
diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h
index 6cc5752457..e8d963b6fb 100644
--- a/include/hw/arm/fsl-imx7.h
+++ b/include/hw/arm/fsl-imx7.h
@@ -175,6 +175,11 @@ enum FslIMX7MemoryMap {
 FSL_IMX7_UART6_ADDR   = 0x30A8,
 FSL_IMX7_UART7_ADDR   = 0x30A9,
 
+FSL_IMX7_SAI1_ADDR= 0x308A,
+FSL_IMX7_SAI2_ADDR= 0x308B,
+FSL_IMX7_SAI3_ADDR= 0x308C,
+FSL_IMX7_SAIn_SIZE= 0x1,
+
 FSL_IMX7_ENET1_ADDR   = 0x30BE,
 FSL_IMX7_ENET2_ADDR   = 0x30BF,
 
-- 
2.25.1

Re: [PATCH for 6.2 44/49] bsd-user: Refactor load_elf_sections and is_target_elf_binary

[Richard Henderson]

On 8/7/21 11:42 AM, Warner Losh wrote:

@@ -332,86 +329,25 @@ static abi_ulong load_elf_interp(struct elfhdr 
*interp_elf_ex,
   */
  error = target_mmap(0, INTERP_MAP_SIZE, PROT_NONE,
  MAP_PRIVATE | MAP_ANON, -1, 0);
-if (error == -1) {
+if (rbase == -1) {
  perror("mmap");
  exit(-1);
  }


Replacement not greedy enough -- rbase is always 0.


+/* Check the elf header and see if this a target elf binary. */
+int is_target_elf_binary(int fd)


This doesn't appear to be used at all.
Which is good, because it's buggy.

Otherwise,
Reviewed-by: Richard Henderson 


r~

Re: [PATCH v3 08/10] virtiofsd: Add inodes_by_handle hash table

[Vivek Goyal]

On Tue, Aug 10, 2021 at 04:13:44PM +0200, Hanna Reitz wrote:
> On 10.08.21 16:07, Vivek Goyal wrote:
> > On Mon, Aug 09, 2021 at 06:47:18PM +0200, Hanna Reitz wrote:
> > > On 09.08.21 18:10, Vivek Goyal wrote:
> > > > On Fri, Jul 30, 2021 at 05:01:32PM +0200, Max Reitz wrote:
> > > > > Currently, lo_inode.fhandle is always NULL and so always keep an 
> > > > > O_PATH
> > > > > FD in lo_inode.fd.  Therefore, when the respective inode is unlinked,
> > > > > its inode ID will remain in use until we drop our lo_inode (and
> > > > > lo_inode_put() thus closes the FD).  Therefore, lo_find() can safely 
> > > > > use
> > > > > the inode ID as an lo_inode key, because any inode with an inode ID we
> > > > > find in lo_data.inodes (on the same filesystem) must be the exact same
> > > > > file.
> > > > > 
> > > > > This will change when we start setting lo_inode.fhandle so we do not
> > > > > have to keep an O_PATH FD open.  Then, unlinking such an inode will
> > > > > immediately remove it, so its ID can then be reused by newly created
> > > > > files, even while the lo_inode object is still there[1].
> > > > > 
> > > > > So creating a new file can then reuse the old file's inode ID, and
> > > > > looking up the new file would lead to us finding the old file's
> > > > > lo_inode, which is not ideal.
> > > > > 
> > > > > Luckily, just as file handles cause this problem, they also solve it: 
> > > > >  A
> > > > > file handle contains a generation ID, which changes when an inode ID 
> > > > > is
> > > > > reused, so the new file can be distinguished from the old one.  So all
> > > > > we need to do is to add a second map besides lo_data.inodes that maps
> > > > > file handles to lo_inodes, namely lo_data.inodes_by_handle.  For
> > > > > clarity, lo_data.inodes is renamed to lo_data.inodes_by_ids.
> > > > > 
> > > > > Unfortunately, we cannot rely on being able to generate file handles
> > > > > every time.  Therefore, we still enter every lo_inode object into
> > > > > inodes_by_ids, but having an entry in inodes_by_handle is optional.  A
> > > > > potential inodes_by_handle entry then has precedence, the 
> > > > > inodes_by_ids
> > > > > entry is just a fallback.
> > > > > 
> > > > > Note that we do not generate lo_fhandle objects yet, and so we also do
> > > > > not enter anything into the inodes_by_handle map yet.  Also, all 
> > > > > lookups
> > > > > skip that map.  We might manually create file handles with some code
> > > > > that is immediately removed by the next patch again, but that would
> > > > > break the assumption in lo_find() that every lo_inode with a non-NULL
> > > > > .fhandle must have an entry in inodes_by_handle and vice versa.  So we
> > > > > leave actually using the inodes_by_handle map for the next patch.
> > > > > 
> > > > > [1] If some application in the guest still has the file open, there is
> > > > > going to be a corresponding FD mapping in lo_data.fd_map.  In such a
> > > > > case, the inode will only go away once every application in the guest
> > > > > has closed it.  The problem described only applies to cases where the
> > > > > guest does not have the file open, and it is just in the dentry cache,
> > > > > basically.
> > > > > 
> > > > > Signed-off-by: Max Reitz 
> > > > > ---
> > > > >tools/virtiofsd/passthrough_ll.c | 81 
> > > > > +---
> > > > >1 file changed, 65 insertions(+), 16 deletions(-)
> > > > > 
> > > > > diff --git a/tools/virtiofsd/passthrough_ll.c 
> > > > > b/tools/virtiofsd/passthrough_ll.c
> > > > > index 487448d666..f9d8b2f134 100644
> > > > > --- a/tools/virtiofsd/passthrough_ll.c
> > > > > +++ b/tools/virtiofsd/passthrough_ll.c
> > > > > @@ -180,7 +180,8 @@ struct lo_data {
> > > > >int announce_submounts;
> > > > >bool use_statx;
> > > > >struct lo_inode root;
> > > > > -GHashTable *inodes; /* protected by lo->mutex */
> > > > > +GHashTable *inodes_by_ids; /* protected by lo->mutex */
> > > > > +GHashTable *inodes_by_handle; /* protected by lo->mutex */
> > > > >struct lo_map ino_map; /* protected by lo->mutex */
> > > > >struct lo_map dirp_map; /* protected by lo->mutex */
> > > > >struct lo_map fd_map; /* protected by lo->mutex */
> > > > > @@ -263,8 +264,9 @@ static struct {
> > > > >/* That we loaded cap-ng in the current thread from the saved */
> > > > >static __thread bool cap_loaded = 0;
> > > > > -static struct lo_inode *lo_find(struct lo_data *lo, struct stat *st,
> > > > > -uint64_t mnt_id);
> > > > > +static struct lo_inode *lo_find(struct lo_data *lo,
> > > > > +const struct lo_fhandle *fhandle,
> > > > > +struct stat *st, uint64_t mnt_id);
> > > > >static int xattr_map_client(const struct lo_data *lo, const char 
> > > > > *client_name,
> > > > >char **out_name);
> > > > > @@ -1064,18 +1066,40 @@ out_err:
> >

RE: [CXL volatile MEM] - Qemu command to turn on HMAT and NUMA fails with assertion

[Samarth Saxena]

Thanks Dave,

The Qemu version is qemu-6.0.50.

I am trying to capture the stack and will place it ASAP.

Regards,
Samarth


-Original Message-
From: Dr. David Alan Gilbert  
Sent: Tuesday, August 10, 2021 4:58 PM
To: Samarth Saxena ; ben.widaw...@intel.com
Cc: qemu-devel@nongnu.org
Subject: Re: [CXL volatile MEM] - Qemu command to turn on HMAT and NUMA fails 
with assertion

EXTERNAL MAIL


* Samarth Saxena (samar...@cadence.com) wrote:
> Hi All,
> 
> I am trying the following command to run Qemu with Kernel 5.14.

cc'ing in Ben who I think owns the CXL stuff.

> qemu-system-x86_64 -M q35,accel=kvm,nvdimm=on,cxl=on,hmat=on -m 
> 8448M,slots=2,maxmem=16G -smp 8,sockets=2,cores=2,threads=2 -hda 
> /lan/dscratch/singhabh/shradha/ubuntu-20.10-64_with_orig_driver_backup
> .qcow2 -enable-kvm -usbdevice tablet -L $VB_ROOT/etc/vm/common/ 
> -object memory-backend-ram,id=cxl-ram,share=on,size=256M -device 
> "pxb-cxl,id=cxl.0,bus=pcie.0,bus_nr=52,uid=0,len-window-base=1,window-
> base[0]=0x4c000,memdev[0]=cxl-ram" -device 
> cxl-rp,id=rp0,bus=cxl.0,addr=0.0,chassis=0,slot=0 -device 
> cxl-type3,bus=rp0,memdev=cxl-ram,id=cxl-vmem0,size=256M -numa 
> node,nodeid=0,memdev=cxl-ram -object 
> memory-backend-ram,id=other-ram,size=8G,prealloc=n,share=off -numa 
> node,nodeid=1,memdev=other-ram,initiator=0 -numa 
> cpu,node-id=0,socket-id=0 -numa cpu,node-id=0,socket-id=1

You probably need to state which qemu tree and version you're using here.

> I get the following crash before the machine starts to boot.
> 
> qemu-system-x86_64: ../softmmu/memory.c:2443: 
> memory_region_add_subregion_common: Assertion `!subregion->container' failed.

It's probably best to check with Ben, but you'll want a backtrace and figure 
out which subregion and region you're dealing with.

Dave

> 
> Please help me find what's wrong here.
> 
> Regards,
> [CadenceLogoRed185Regcopy1583174817new51584636989.png] ence.com/en_US/home.html>
> Samarth Saxena
> Sr Principal Software Engineer
> T: +911204308300
> [UIcorrectsize1583179003.png]
> [16066EmailSignatureFortune100Best2021White92x1271617625037.png] ://www.cadence.com/en_US/home/company/careers.html>
> 
> 
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH] build: remove redundant 'check-build' make target

[Willian Rampazzo]

On Tue, Aug 10, 2021 at 1:44 PM Daniel P. Berrangé  wrote:
>
> The 'check-build' make target was added as a way to build all the unit
> test binaries, since the standard 'all' target would not trigger this.
>
> Since the switch to meson, however, 'all' will now include the 'test'
> binaries. As a result, 'check-build' is a no-op:
>
>$ make all check-build
>..snip lots of output...
>make: Nothing to be done for 'check-build'.
>

Confirmed!

> Signed-off-by: Daniel P. Berrangé 
> ---
>  .gitlab-ci.d/buildtest.yml   | 9 -
>  .gitlab-ci.d/crossbuild-template.yml | 6 +++---
>  tests/Makefile.include   | 3 ---
>  3 files changed, 3 insertions(+), 15 deletions(-)
>

Reviewed-by: Willian Rampazzo 

Re: [PATCH for 6.2 43/49] bsd-user: elfload.c style catch up patch

[Richard Henderson]

On 8/7/21 11:42 AM, Warner Losh wrote:

From: Warner Losh

Various style fixes to elfload.c that were too painful to make earlier
in this series.

Signed-off-by: Warner Losh

Sponsored by:   Netflix
---
  bsd-user/elfload.c | 212 ++---
  1 file changed, 105 insertions(+), 107 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for 6.2 42/49] bsd-user: add stubbed out core dump support

[Richard Henderson]

On 8/7/21 11:42 AM, Warner Losh wrote:

From: Warner Losh 

Add a stubbed-out version of the bsd-user fork's core dump support. This
allows elfload.c to be almost the same between what's upstream and
what's in qemu-project upstream w/o the burden of reviewing the core
dump support.

Signed-off-by: Stacey Son 
Signed-off-by: Warner Losh 

Sponsored by:   Netflix
---
  bsd-user/elfcore.c | 10 ++
  bsd-user/elfload.c | 24 ++--
  bsd-user/qemu.h|  6 ++
  3 files changed, 38 insertions(+), 2 deletions(-)
  create mode 100644 bsd-user/elfcore.c

diff --git a/bsd-user/elfcore.c b/bsd-user/elfcore.c
new file mode 100644
index 00..e3c161942d
--- /dev/null
+++ b/bsd-user/elfcore.c
@@ -0,0 +1,10 @@
+/* Stubbed out version of core dump support, explicitly in public domain */
+
+static int elf_core_dump(int signr, CPUArchState *env)
+{
+struct elf_note en;
+
+bswap_note(&en);
+
+return 0;
+}


No warnings from this, e.g. uninitialized variable?
If you have a chance, run this through clang-12 (mainline).
It it getting much better at identifying such stuff.

Otherwise,
Reviewed-by: Richard Henderson 


r~

[PATCH qemu 0/1] Document how to use gdbserver with unix sockets

[~archi42]

I had some trouble with gdb over unix sockets, and Peter was so nice to
help me out with it on IRC.
This adds some docs for that, so others
might have less trouble:

< pm215> yeah, that would be good

Feel
free to adjust/rephrase/rearrange the patch :)

Sebastian Meyer (1):
  docs: how to use gdb with unix sockets

 docs/system/gdb.rst | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

-- 
2.32.0

[PATCH qemu 1/1] docs: how to use gdb with unix sockets

[~archi42]

From: Sebastian Meyer 

Signed-off-by: Sebastian Meyer 
---
 docs/system/gdb.rst | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/docs/system/gdb.rst b/docs/system/gdb.rst
index 144d083df3..2ff4d6aab5 100644
--- a/docs/system/gdb.rst
+++ b/docs/system/gdb.rst
@@ -15,7 +15,8 @@ The ``-s`` option will make QEMU listen for an incoming 
connection
 from gdb on TCP port 1234, and ``-S`` will make QEMU not start the
 guest until you tell it to from gdb. (If you want to specify which
 TCP port to use or to use something other than TCP for the gdbstub
-connection, use the ``-gdb dev`` option instead of ``-s``.)
+connection, use the ``-gdb dev`` option instead of ``-s``. See
+further below for an example using unix sockets.)
 
 .. parsed-literal::
 
@@ -168,3 +169,24 @@ The memory mode can be checked by sending the following 
command:
 
 ``maintenance packet Qqemu.PhyMemMode:0``
 This will change it back to normal memory mode.
+
+Using unix sockets
+^^
+
+An alternate method for connecting gdb to the qemu gdbstub are unix
+sockets (if supported by your operating system). This is useful when
+running serveral tests in parallel and/or you do not know a free tcp
+port a priori (e.g. when running automated tests).
+First create a new chardev with the appropriate options, then
+instruct the gdbserver to use that device::
+
+.. parsed-literal::
+
+   |qemu_system| -chardev 
socket,path=/tmp/gdb-socket,server=on,wait=off,id=gdb0 -gdb chardev:gdb0 -S 
-kernel bzImage -hda rootdisk.img -append "root=/dev/hda"
+
+Start gdb as before, but this time connect using the path to
+the socket::
+
+   (gdb) target remote /tmp/gdb-socket
+
+Please mind that this usually requires gdb version 9.0 or newer.
-- 
2.32.0

Re: [PATCH v3 06/13] python/aqmp-tui: Added type annotations for aqmp-tui

[Niteesh G. S.]

On Fri, Aug 6, 2021 at 1:26 AM John Snow  wrote:

>
>
> On Fri, Jul 30, 2021 at 4:19 PM G S Niteesh Babu 
> wrote:
>
>> This patch adds type annotations for aqmp-tui using
>> the mypy library.
>>
>>
> Awesome, thanks for taking a swing at this. Looks like it wasn't as bad as
> I was fearing.
>
>
>> Signed-off-by: G S Niteesh Babu 
>> ---
>>  python/qemu/aqmp/aqmp_tui.py | 79 
>>  python/setup.cfg |  3 --
>>  2 files changed, 43 insertions(+), 39 deletions(-)
>>
>> diff --git a/python/qemu/aqmp/aqmp_tui.py b/python/qemu/aqmp/aqmp_tui.py
>> index ec9eba0aa7..ab9ada793a 100644
>> --- a/python/qemu/aqmp/aqmp_tui.py
>> +++ b/python/qemu/aqmp/aqmp_tui.py
>> @@ -9,8 +9,15 @@
>>  import argparse
>>  import asyncio
>>  import logging
>> -from logging import Handler
>> +from logging import Handler, LogRecord
>>  import signal
>> +from typing import (
>> +Any,
>> +List,
>> +Optional,
>> +Tuple,
>> +Union,
>> +)
>>
>>  import urwid
>>  import urwid_readline
>> @@ -22,13 +29,13 @@
>>  from .util import create_task, pretty_traceback
>>
>>
>> -UPDATE_MSG = 'UPDATE_MSG'
>> +UPDATE_MSG: str = 'UPDATE_MSG'
>>
>>  # Using root logger to enable all loggers under qemu and asyncio
>>  LOGGER = logging.getLogger()
>>
>>
>> -def format_json(msg):
>> +def format_json(msg: str) -> str:
>>  """
>>  Formats given multiline JSON message into a single line message.
>>  Converting into single line is more asthetically pleasing when
>> looking
>> @@ -43,17 +50,17 @@ def format_json(msg):
>>
>>
>>  class App(QMPClient):
>> -def __init__(self, address):
>> +def __init__(self, address: Union[str, Tuple[str, int]]) -> None:
>>  urwid.register_signal(type(self), UPDATE_MSG)
>>  self.window = Window(self)
>>  self.address = address
>> -self.aloop = None
>> +self.aloop: Optional[Any] = None  # FIXME: Use more concrete
>> type.
>>
>
> I ran into this in util.py; you want Optional[asyncio.AbstractEventLoop]
> here.
>
Thanks. Fixed.

>
>
>>  super().__init__()
>>
>> -def add_to_history(self, msg):
>> +def add_to_history(self, msg: str) -> None:
>>  urwid.emit_signal(self, UPDATE_MSG, msg)
>>
>> -def _cb_outbound(self, msg):
>> +def _cb_outbound(self, msg: Message) -> Message:
>>  # FIXME: I think the ideal way to omit these messages during
>> in-TUI
>>  # logging will be to add a filter to the logger. We can use
>> regex to
>>  # filter out messages starting with 'Request:' or 'Response:'
>> but I
>> @@ -67,25 +74,25 @@ def _cb_outbound(self, msg):
>>  self.add_to_history('<-- ' + str(msg))
>>  return msg
>>
>> -def _cb_inbound(self, msg):
>> +def _cb_inbound(self, msg: Message) -> Message:
>>  handler = LOGGER.handlers[0]
>>  if not isinstance(handler, TUILogHandler):
>>  LOGGER.debug('Response: %s', str(msg))
>>  self.add_to_history('--> ' + str(msg))
>>  return msg
>>
>> -async def wait_for_events(self):
>> +async def wait_for_events(self) -> None:
>>  async for event in self.events:
>>  self.handle_event(event)
>>
>> -async def _send_to_server(self, raw_msg):
>> +async def _send_to_server(self, raw_msg: str) -> None:
>>  # FIXME: Format the raw_msg in history view to one line. It is
>> not
>>  # pleasing to see multiple lines JSON object with an error
>> statement.
>>  try:
>>  msg = Message(bytes(raw_msg, encoding='utf-8'))
>>  # Format multiline json into a single line JSON, since it is
>> more
>>  # pleasing to look along with err message in TUI.
>> -raw_msg = self.format_json(raw_msg)
>> +raw_msg = format_json(raw_msg)
>>  await self._raw(msg, assign_id='id' not in msg)
>>  except (ValueError, TypeError) as err:
>>  LOGGER.info('Invalid message: %s', str(err))
>> @@ -102,18 +109,18 @@ def _cb_inbound(self, msg):
>>  LOGGER.error('Exception from _send_to_server: %s', str(err))
>>  raise err
>>
>> -def cb_send_to_server(self, msg):
>> +def cb_send_to_server(self, msg: str) -> None:
>>  create_task(self._send_to_server(msg))
>>
>> -def unhandled_input(self, key):
>> +def unhandled_input(self, key: str) -> None:
>>  if key == 'esc':
>>  self.kill_app()
>>
>> -def kill_app(self):
>> +def kill_app(self) -> None:
>>  # TODO: Work on the disconnect logic
>>  create_task(self._kill_app())
>>
>> -async def _kill_app(self):
>> +async def _kill_app(self) -> None:
>>  # It is ok to call disconnect even in disconnect state
>>  try:
>>  await self.disconnect()
>> @@ -124,7 +131,7 @@ def kill_app(self):
>>  raise err
>>  raise urwid.ExitMainLoop()
>>
>> -def handle_event(self, event):
>> +def handle_event(

Re: [PATCH V6 19/27] vfio-pci: cpr part 1 (fd and dma)

[Alex Williamson]

On Fri,  6 Aug 2021 14:43:53 -0700
Steve Sistare  wrote:

> Enable vfio-pci devices to be saved and restored across an exec restart
> of qemu.
> 
> At vfio creation time, save the value of vfio container, group, and device
> descriptors in cpr state.
> 
> In cpr-save and cpr-exec, suspend the use of virtual addresses in DMA
> mappings with VFIO_DMA_UNMAP_FLAG_VADDR, because guest ram will be remapped
> at a different VA after exec.  DMA to already-mapped pages continues.  Save
> the msi message area as part of vfio-pci vmstate, save the interrupt and
> notifier eventfd's in cpr state, and clear the close-on-exec flag for the
> vfio descriptors.  The flag is not cleared earlier because the descriptors
> should not persist across miscellaneous fork and exec calls that may be
> performed during normal operation.
> 
> On qemu restart, vfio_realize() finds the descriptor env vars, uses
> the descriptors, and notes that the device is being reused.  Device and
> iommu state is already configured, so operations in vfio_realize that
> would modify the configuration are skipped for a reused device, including
> vfio ioctl's and writes to PCI configuration space.  The result is that
> vfio_realize constructs qemu data structures that reflect the current
> state of the device.  However, the reconstruction is not complete until
> cpr-load is called. cpr-load loads the msi data and finds eventfds in cpr
> state.  It rebuilds vector data structures and attaches the interrupts to
> the new KVM instance.  cpr-load then walks the flattened ranges of the
> vfio_address_spaces and calls VFIO_DMA_MAP_FLAG_VADDR to inform the kernel
> of the new VA's.  Lastly, it starts the VM and suppresses vfio device reset.
> 
> This functionality is delivered by 3 patches for clarity.  Part 1 handles
> device file descriptors and DMA.  Part 2 adds eventfd and MSI/MSI-X vector
> support.  Part 3 adds INTX support.
> 
> Signed-off-by: Steve Sistare 
> ---
>  MAINTAINERS   |   1 +
>  hw/pci/pci.c  |   4 ++
>  hw/vfio/common.c  |  69 --
>  hw/vfio/cpr.c | 160 
> ++
>  hw/vfio/meson.build   |   1 +
>  hw/vfio/pci.c |  57 +++
>  hw/vfio/trace-events  |   1 +
>  include/hw/pci/pci.h  |   1 +
>  include/hw/vfio/vfio-common.h |   5 ++
>  include/migration/cpr.h   |   3 +
>  linux-headers/linux/vfio.h|   6 ++
>  migration/cpr.c   |  10 ++-
>  migration/target.c|  14 
>  13 files changed, 325 insertions(+), 7 deletions(-)
>  create mode 100644 hw/vfio/cpr.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a9d2ed8..3132965 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2904,6 +2904,7 @@ CPR
>  M: Steve Sistare 
>  M: Mark Kanda 
>  S: Maintained
> +F: hw/vfio/cpr.c
>  F: include/migration/cpr.h
>  F: migration/cpr.c
>  F: qapi/cpr.json
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 59408a3..b9c6ca1 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -307,6 +307,10 @@ static void pci_do_device_reset(PCIDevice *dev)
>  {
>  int r;
>  
> +if (dev->reused) {
> +return;
> +}
> +
>  pci_device_deassert_intx(dev);
>  assert(dev->irq_state == 0);
>  
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 7918c0d..872a1ac 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -31,6 +31,7 @@
>  #include "exec/memory.h"
>  #include "exec/ram_addr.h"
>  #include "hw/hw.h"
> +#include "migration/cpr.h"
>  #include "qemu/error-report.h"
>  #include "qemu/main-loop.h"
>  #include "qemu/range.h"
> @@ -464,6 +465,10 @@ static int vfio_dma_unmap(VFIOContainer *container,
>  return vfio_dma_unmap_bitmap(container, iova, size, iotlb);
>  }
>  
> +if (container->reused) {
> +return 0;
> +}
> +
>  while (ioctl(container->fd, VFIO_IOMMU_UNMAP_DMA, &unmap)) {
>  /*
>   * The type1 backend has an off-by-one bug in the kernel 
> (71a7d3d78e3c
> @@ -501,6 +506,10 @@ static int vfio_dma_map(VFIOContainer *container, hwaddr 
> iova,
>  .size = size,
>  };
>  
> +if (container->reused) {
> +return 0;
> +}
> +
>  if (!readonly) {
>  map.flags |= VFIO_DMA_MAP_FLAG_WRITE;
>  }
> @@ -1872,6 +1881,10 @@ static int vfio_init_container(VFIOContainer 
> *container, int group_fd,
>  if (iommu_type < 0) {
>  return iommu_type;
>  }
> +if (container->reused) {
> +container->iommu_type = iommu_type;
> +return 0;
> +}
>  

I'd like to see more comments throughout, but particularly where we're
dumping out of functions for reused containers, groups, and devices.
For instance map/unmap we're assuming we'll reach the same IOMMU
mapping state we had previously, how do we validate that, why can't we
only set vaddr in the mapping path rather than skipping it for a later
pass at the flatmap, do we actually see unmaps, is

Re: [PATCH V6 18/27] vfio-pci: refactor for cpr

[Alex Williamson]

On Fri,  6 Aug 2021 14:43:52 -0700
Steve Sistare  wrote:

> Export vfio_address_spaces and vfio_listener_skipped_section.
> Add optional name arg to vfio_add_kvm_msi_virq.
> Refactor vector use into a helper vfio_vector_init.
> All for use by cpr in a subsequent patch.  No functional change.

Why is the name arg optional?  It seems really inconsistent to me that
everything other than MSI/X uses this with a name, but MSI/X use NULL
and in an entirely separate pre-save step we then iterate through all
the {event,irq}fds to save them.  If we asked for a named notifier,
shouldn't we go ahead and save it under that name at that time?  ie.

static int vfio_named_notifier_init(VFIOPCIDevice *vdev, EventNotifier *e,
const char *name, int nr)
{
int ret, fd = load_event_fd(vdev, name, nr);

if (fd >= 0) {
event_notifier_init_fd(e, fd);
} else {
ret = event_notifier_init(e, 0);
if (ret) {
return ret;
}
save_event_fd(vdev, name, nr, e);
}
return 0;
}

Are we not doing this to avoid runtime overhead?

In the process, maybe we can use more descriptive names than
"interrupt", ex. "msi" or "msix".

It also feels a bit forced to me that the entire fd saving uses {name,
id} but vfio is the only caller that makes use of a non-zero id.
Should we instead just wrap all the calls from vfio to append the id to
the name so the common code can just use strcmp()?  Thanks,

Alex

Re: [PATCH for 6.2 41/49] bsd-user: Add target_os_user.h to capture the user/kernel structures

[Richard Henderson]

On 8/7/21 11:42 AM, Warner Losh wrote:

From: Warner Losh

This file evolved over the years to capture the user/kernel interfaces,
including those that changed over time.

Signed-off-by: Stacey Son
Signed-off-by: Michal Meloun
Signed-off-by: Warner Losh
---
  bsd-user/freebsd/target_os_user.h | 429 ++
  1 file changed, 429 insertions(+)
  create mode 100644 bsd-user/freebsd/target_os_user.h


Acked-by: Richard Henderson 


r~

Re: [PATCH for 6.2 40/49] bsd-user: Add target_arch_reg to describe a target's register set

[Richard Henderson]

On 8/7/21 11:42 AM, Warner Losh wrote:

From: Warner Losh

target_reg_t is the normal register. target_fpreg_t is the floating
point registers. target_copy_regs copies the registers out of CPU
context for things like core dumps.

Signed-off-by: Stacey Son
Signed-off-by: Warner Losh
---
  bsd-user/i386/target_arch_reg.h   | 82 +++
  bsd-user/x86_64/target_arch_reg.h | 92 +++
  2 files changed, 174 insertions(+)
  create mode 100644 bsd-user/i386/target_arch_reg.h
  create mode 100644 bsd-user/x86_64/target_arch_reg.h


Reviewed-by: Richard Henderson 

While this mirrors what linux-user does, I've wondered if this wasn't just pointless 
copying.  If a bit of code knows enough about a target to fill in its core dump, why 
wouldn't it just copy the data straight from CPUArchState instead of using these 
intermediaries?



r~

[PATCH] build: remove redundant 'check-build' make target

[Daniel P . Berrangé]

The 'check-build' make target was added as a way to build all the unit
test binaries, since the standard 'all' target would not trigger this.

Since the switch to meson, however, 'all' will now include the 'test'
binaries. As a result, 'check-build' is a no-op:

   $ make all check-build
   ..snip lots of output...
   make: Nothing to be done for 'check-build'.

Signed-off-by: Daniel P. Berrangé 
---
 .gitlab-ci.d/buildtest.yml   | 9 -
 .gitlab-ci.d/crossbuild-template.yml | 6 +++---
 tests/Makefile.include   | 3 ---
 3 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 903ee65f32..a210d5f2e6 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -9,7 +9,6 @@ build-system-alpine:
 IMAGE: alpine
 TARGETS: aarch64-softmmu alpha-softmmu cris-softmmu hppa-softmmu
   microblazeel-softmmu mips64el-softmmu
-MAKE_CHECK_ARGS: check-build
 CONFIGURE_ARGS: --enable-docs --enable-trace-backends=log,simple,syslog
   artifacts:
 expire_in: 2 days
@@ -44,7 +43,6 @@ build-system-ubuntu:
 CONFIGURE_ARGS: --enable-docs --enable-fdt=system --enable-slirp=system
 TARGETS: aarch64-softmmu alpha-softmmu cris-softmmu hppa-softmmu
   microblazeel-softmmu mips64el-softmmu
-MAKE_CHECK_ARGS: check-build
   artifacts:
 expire_in: 2 days
 paths:
@@ -77,7 +75,6 @@ build-system-debian:
 CONFIGURE_ARGS: --enable-fdt=system
 TARGETS: arm-softmmu avr-softmmu i386-softmmu mipsel-softmmu
   riscv64-softmmu sh4eb-softmmu sparc-softmmu xtensaeb-softmmu
-MAKE_CHECK_ARGS: check-build
   artifacts:
 expire_in: 2 days
 paths:
@@ -111,7 +108,6 @@ build-system-fedora:
  --enable-fdt=system --enable-slirp=system --enable-capstone=system
 TARGETS: tricore-softmmu microblaze-softmmu mips-softmmu
   xtensa-softmmu m68k-softmmu riscv32-softmmu ppc-softmmu sparc64-softmmu
-MAKE_CHECK_ARGS: check-build
   artifacts:
 expire_in: 2 days
 paths:
@@ -145,7 +141,6 @@ build-system-centos:
 --enable-modules --enable-trace-backends=dtrace
 TARGETS: ppc64-softmmu or1k-softmmu s390x-softmmu
   x86_64-softmmu rx-softmmu sh4-softmmu nios2-softmmu
-MAKE_CHECK_ARGS: check-build
   artifacts:
 expire_in: 2 days
 paths:
@@ -177,7 +172,6 @@ build-system-opensuse:
 IMAGE: opensuse-leap
 CONFIGURE_ARGS: --enable-fdt=system
 TARGETS: s390x-softmmu x86_64-softmmu aarch64-softmmu
-MAKE_CHECK_ARGS: check-build
   artifacts:
 expire_in: 2 days
 paths:
@@ -410,7 +404,6 @@ build-cfi-aarch64:
 CONFIGURE_ARGS: --cc=clang --cxx=clang++ --enable-cfi --enable-cfi-debug
   --enable-safe-stack --enable-slirp=git
 TARGETS: aarch64-softmmu
-MAKE_CHECK_ARGS: check-build
   timeout: 70m
   artifacts:
 expire_in: 2 days
@@ -452,7 +445,6 @@ build-cfi-ppc64-s390x:
 CONFIGURE_ARGS: --cc=clang --cxx=clang++ --enable-cfi --enable-cfi-debug
   --enable-safe-stack --enable-slirp=git
 TARGETS: ppc64-softmmu s390x-softmmu
-MAKE_CHECK_ARGS: check-build
   timeout: 70m
   artifacts:
 expire_in: 2 days
@@ -494,7 +486,6 @@ build-cfi-x86_64:
 CONFIGURE_ARGS: --cc=clang --cxx=clang++ --enable-cfi --enable-cfi-debug
   --enable-safe-stack --enable-slirp=git
 TARGETS: x86_64-softmmu
-MAKE_CHECK_ARGS: check-build
   timeout: 70m
   artifacts:
 expire_in: 2 days
diff --git a/.gitlab-ci.d/crossbuild-template.yml 
b/.gitlab-ci.d/crossbuild-template.yml
index 7d3ad00a1e..03a6d42980 100644
--- a/.gitlab-ci.d/crossbuild-template.yml
+++ b/.gitlab-ci.d/crossbuild-template.yml
@@ -10,7 +10,7 @@
 --disable-user --target-list-exclude="arm-softmmu cris-softmmu
   i386-softmmu microblaze-softmmu mips-softmmu mipsel-softmmu
   mips64-softmmu ppc-softmmu sh4-softmmu xtensa-softmmu"
-- make -j$(expr $(nproc) + 1) all check-build $MAKE_CHECK_ARGS
+- make -j$(expr $(nproc) + 1) all $MAKE_CHECK_ARGS
 - if grep -q "EXESUF=.exe" config-host.mak;
   then make installer;
   version="$(git describe --match v[0-9]*)";
@@ -32,7 +32,7 @@
 - PKG_CONFIG_PATH=$PKG_CONFIG_PATH
   ../configure --enable-werror --disable-docs $QEMU_CONFIGURE_OPTS
 --disable-tools --enable-${ACCEL:-kvm} $EXTRA_CONFIGURE_OPTS
-- make -j$(expr $(nproc) + 1) all check-build $MAKE_CHECK_ARGS
+- make -j$(expr $(nproc) + 1) all $MAKE_CHECK_ARGS
 
 .cross_user_build_job:
   stage: build
@@ -43,4 +43,4 @@
 - PKG_CONFIG_PATH=$PKG_CONFIG_PATH
   ../configure --enable-werror --disable-docs $QEMU_CONFIGURE_OPTS
 --disable-system
-- make -j$(expr $(nproc) + 1) all check-build $MAKE_CHECK_ARGS
+- make -j$(expr $(nproc) + 1) all $MAKE_CHECK_ARGS
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 6e16c05f10..fa08bf3888 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -23,7 +23,6 @@ endif
@echo " $(MAK

Re: [PATCH v2] fsl-imx6ul: Instantiate SAI1/2/3 and ASRC as unimplemented devices

[Philippe Mathieu-Daudé]

On 8/10/21 6:03 PM, Guenter Roeck wrote:
> Instantiate SAI1/2/3 and ASRC as unimplemented devices to avoid random
> Linux kernel crashes, such as
> 
> Unhandled fault: external abort on non-linefetch (0x808) at 0xd1580010^M
> pgd = (ptrval)^M
> [d1580010] *pgd=8231b811, *pte=02034653, *ppte=02034453^M
> Internal error: : 808 [#1] SMP ARM^M
> ...
> [] (regmap_mmio_write32le) from [] 
> (regmap_mmio_write+0x3c/0x54)^M
> [] (regmap_mmio_write) from [] 
> (_regmap_write+0x4c/0x1f0)^M
> [] (_regmap_write) from [] 
> (_regmap_update_bits+0xe4/0xec)^M
> [] (_regmap_update_bits) from [] 
> (regmap_update_bits_base+0x50/0x74)^M
> [] (regmap_update_bits_base) from [] 
> (fsl_asrc_runtime_resume+0x1e4/0x21c)^M
> [] (fsl_asrc_runtime_resume) from [] 
> (__rpm_callback+0x3c/0x108)^M
> [] (__rpm_callback) from [] (rpm_callback+0x60/0x64)^M
> [] (rpm_callback) from [] (rpm_resume+0x5cc/0x808)^M
> [] (rpm_resume) from [] (__pm_runtime_resume+0x60/0xa0)^M
> [] (__pm_runtime_resume) from [] 
> (fsl_asrc_probe+0x2a8/0x708)^M
> [] (fsl_asrc_probe) from [] (platform_probe+0x58/0xb8)^M
> [] (platform_probe) from [] 
> (really_probe.part.0+0x9c/0x334)^M
> [] (really_probe.part.0) from [] 
> (__driver_probe_device+0xa0/0x138)^M
> [] (__driver_probe_device) from [] 
> (driver_probe_device+0x30/0xc8)^M
> [] (driver_probe_device) from [] 
> (__driver_attach+0x90/0x130)^M
> [] (__driver_attach) from [] 
> (bus_for_each_dev+0x78/0xb8)^M
> [] (bus_for_each_dev) from [] 
> (bus_add_driver+0xf0/0x1d8)^M
> [] (bus_add_driver) from [] (driver_register+0x88/0x118)^M
> [] (driver_register) from [] 
> (do_one_initcall+0x7c/0x3a4)^M
> [] (do_one_initcall) from [] 
> (kernel_init_freeable+0x198/0x22c)^M
> [] (kernel_init_freeable) from [] 
> (kernel_init+0x10/0x128)^M
> [] (kernel_init) from [] (ret_from_fork+0x14/0x38)^M
> 
> or
> 
> Unhandled fault: external abort on non-linefetch (0x808) at 0xd19b^M
> pgd = (ptrval)^M
> [d19b] *pgd=82711811, *pte=308a0653, *ppte=308a0453^M
> Internal error: : 808 [#1] SMP ARM^M
> ...
> [] (regmap_mmio_write32le) from [] 
> (regmap_mmio_write+0x3c/0x54)^M
> [] (regmap_mmio_write) from [] 
> (_regmap_write+0x4c/0x1f0)^M
> [] (_regmap_write) from [] (regmap_write+0x3c/0x60)^M
> [] (regmap_write) from [] 
> (fsl_sai_runtime_resume+0x9c/0x1ec)^M
> [] (fsl_sai_runtime_resume) from [] 
> (__rpm_callback+0x3c/0x108)^M
> [] (__rpm_callback) from [] (rpm_callback+0x60/0x64)^M
> [] (rpm_callback) from [] (rpm_resume+0x5cc/0x808)^M
> [] (rpm_resume) from [] (__pm_runtime_resume+0x60/0xa0)^M
> [] (__pm_runtime_resume) from [] 
> (fsl_sai_probe+0x2b8/0x65c)^M
> [] (fsl_sai_probe) from [] (platform_probe+0x58/0xb8)^M
> [] (platform_probe) from [] 
> (really_probe.part.0+0x9c/0x334)^M
> [] (really_probe.part.0) from [] 
> (__driver_probe_device+0xa0/0x138)^M
> [] (__driver_probe_device) from [] 
> (driver_probe_device+0x30/0xc8)^M
> [] (driver_probe_device) from [] 
> (__driver_attach+0x90/0x130)^M
> [] (__driver_attach) from [] 
> (bus_for_each_dev+0x78/0xb8)^M
> [] (bus_for_each_dev) from [] 
> (bus_add_driver+0xf0/0x1d8)^M
> [] (bus_add_driver) from [] (driver_register+0x88/0x118)^M
> [] (driver_register) from [] 
> (do_one_initcall+0x7c/0x3a4)^M
> [] (do_one_initcall) from [] 
> (kernel_init_freeable+0x198/0x22c)^M
> [] (kernel_init_freeable) from [] 
> (kernel_init+0x10/0x128)^M
> [] (kernel_init) from [] (ret_from_fork+0x14/0x38)^M
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Guenter Roeck 
> ---
> v2: Updated description to include tracebacks and to use the term 
> 'instantiate'
> Added inline comments describing devices in more detail

Thank you!

Peter you might want to s/^M// when queueing ;)

> Added Philippe's Reviewed-by: tag
> 
>  hw/arm/fsl-imx6ul.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
> index e0128d7316..1d1a708dd9 100644
> --- a/hw/arm/fsl-imx6ul.c
> +++ b/hw/arm/fsl-imx6ul.c
> @@ -534,6 +534,13 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error 
> **errp)
>   */
>  create_unimplemented_device("sdma", FSL_IMX6UL_SDMA_ADDR, 0x4000);
>  
> +/*
> + * SAI (Audio SSI (Synchronous Serial Interface))
> + */
> +create_unimplemented_device("sai1", FSL_IMX6UL_SAI1_ADDR, 0x4000);
> +create_unimplemented_device("sai2", FSL_IMX6UL_SAI2_ADDR, 0x4000);
> +create_unimplemented_device("sai3", FSL_IMX6UL_SAI3_ADDR, 0x4000);
> +
>  /*
>   * PWM
>   */
> @@ -542,6 +549,11 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error 
> **errp)
>  create_unimplemented_device("pwm3", FSL_IMX6UL_PWM3_ADDR, 0x4000);
>  create_unimplemented_device("pwm4", FSL_IMX6UL_PWM4_ADDR, 0x4000);
>  
> +/*
> + * Audio ASRC (asynchronous sample rate converter)
> + */
> +create_unimplemented_device("asrc", FSL_IMX6UL_ASRC_ADDR, 0x4000);
> +
>  /*
>   * CAN
>   */
> 

Re: [PATCH for 6.2 39/49] bsd-user: Need to reset CPU after creation.

[Richard Henderson]

On 8/7/21 11:42 AM, Warner Losh wrote:

From: Warner Losh

Signed-off-by: Warner Losh
---
  bsd-user/main.c | 2 ++
  1 file changed, 2 insertions(+)


Reviewed-by: Richard Henderson 

Slightly amusing that the line was removed in patch 1, because it was ifdefed.


r~

Re: [PATCH for 6.2 38/49] bsd-user: Update mapping to handle reserved and starting conditions

[Richard Henderson]

On 8/7/21 11:42 AM, Warner Losh wrote:

From: Warner Losh 

Update the reserved base based on what platform we're on, as well as the
start of the mmap range. Update routines that find va ranges to interact
with the reserved ranges as well as properly align the mapping (this is
especially important for targets whose page size does not match the
host's). Loop where appropriate when the initial address space offered
by mmap does not meet the contraints.

Signed-off-by: Mikaël Urankar 
Signed-off-by: Stacey Son 
Signed-off-by: Warner Losh 
---
  bsd-user/main.c |  23 ++-
  bsd-user/mmap.c | 372 
  bsd-user/qemu.h |   5 +-
  3 files changed, 335 insertions(+), 65 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index 93ef9298b8..36852604f8 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -49,12 +49,29 @@
  #include "target_arch_cpu.h"
  
  int singlestep;

-unsigned long mmap_min_addr;
  uintptr_t guest_base;
  static const char *cpu_model;
  static const char *cpu_type;
  bool have_guest_base;
+#if (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
+/*
+ * When running 32-on-64 we should make sure we can fit all of the possible
+ * guest address space into a contiguous chunk of virtual host memory.
+ *
+ * This way we will never overlap with our own libraries or binaries or stack
+ * or anything else that QEMU maps.
+ */
+# ifdef TARGET_MIPS
+/* MIPS only supports 31 bits of virtual address space for user space */
+unsigned long reserved_va = 0x7700;
+# elif defined(TARGET_PPC64)
+unsigned long reserved_va = 0xf000;
+# else
+unsigned long reserved_va = 0xf700;
+# endif
+#else
  unsigned long reserved_va;
+#endif


All of these 7's look to be copying an old linux-user bug.
I cleaned this up in 18e80c55bb6.

Otherwise,
Acked-by: Richard Henderson 

I would hope one day this memory management could be shared between the user-only 
implementations.  It's complicated enough...



r~

Re: [PATCH for 6.2 37/49] bsd-user: update debugging in mmap.c

[Richard Henderson]

On 8/7/21 11:42 AM, Warner Losh wrote:

From: Warner Losh

Update the debugging code for new features and different targets.

Signed-off-by: Mikaël Urankar
Signed-off-by: Sean Bruno
Signed-off-by: Kyle Evans
Signed-off-by: Warner Losh
---
  bsd-user/mmap.c | 49 ++---
  1 file changed, 30 insertions(+), 19 deletions(-)


Acked-by: Richard Henderson 

For the to-do list: convert to qemu_log_mask or tracepoints.


r~

[PATCH v2] fsl-imx6ul: Instantiate SAI1/2/3 and ASRC as unimplemented devices

[Guenter Roeck]

Instantiate SAI1/2/3 and ASRC as unimplemented devices to avoid random
Linux kernel crashes, such as

Unhandled fault: external abort on non-linefetch (0x808) at 0xd1580010^M
pgd = (ptrval)^M
[d1580010] *pgd=8231b811, *pte=02034653, *ppte=02034453^M
Internal error: : 808 [#1] SMP ARM^M
...
[] (regmap_mmio_write32le) from [] 
(regmap_mmio_write+0x3c/0x54)^M
[] (regmap_mmio_write) from [] (_regmap_write+0x4c/0x1f0)^M
[] (_regmap_write) from [] (_regmap_update_bits+0xe4/0xec)^M
[] (_regmap_update_bits) from [] 
(regmap_update_bits_base+0x50/0x74)^M
[] (regmap_update_bits_base) from [] 
(fsl_asrc_runtime_resume+0x1e4/0x21c)^M
[] (fsl_asrc_runtime_resume) from [] 
(__rpm_callback+0x3c/0x108)^M
[] (__rpm_callback) from [] (rpm_callback+0x60/0x64)^M
[] (rpm_callback) from [] (rpm_resume+0x5cc/0x808)^M
[] (rpm_resume) from [] (__pm_runtime_resume+0x60/0xa0)^M
[] (__pm_runtime_resume) from [] 
(fsl_asrc_probe+0x2a8/0x708)^M
[] (fsl_asrc_probe) from [] (platform_probe+0x58/0xb8)^M
[] (platform_probe) from [] 
(really_probe.part.0+0x9c/0x334)^M
[] (really_probe.part.0) from [] 
(__driver_probe_device+0xa0/0x138)^M
[] (__driver_probe_device) from [] 
(driver_probe_device+0x30/0xc8)^M
[] (driver_probe_device) from [] 
(__driver_attach+0x90/0x130)^M
[] (__driver_attach) from [] (bus_for_each_dev+0x78/0xb8)^M
[] (bus_for_each_dev) from [] (bus_add_driver+0xf0/0x1d8)^M
[] (bus_add_driver) from [] (driver_register+0x88/0x118)^M
[] (driver_register) from [] (do_one_initcall+0x7c/0x3a4)^M
[] (do_one_initcall) from [] 
(kernel_init_freeable+0x198/0x22c)^M
[] (kernel_init_freeable) from [] (kernel_init+0x10/0x128)^M
[] (kernel_init) from [] (ret_from_fork+0x14/0x38)^M

or

Unhandled fault: external abort on non-linefetch (0x808) at 0xd19b^M
pgd = (ptrval)^M
[d19b] *pgd=82711811, *pte=308a0653, *ppte=308a0453^M
Internal error: : 808 [#1] SMP ARM^M
...
[] (regmap_mmio_write32le) from [] 
(regmap_mmio_write+0x3c/0x54)^M
[] (regmap_mmio_write) from [] (_regmap_write+0x4c/0x1f0)^M
[] (_regmap_write) from [] (regmap_write+0x3c/0x60)^M
[] (regmap_write) from [] 
(fsl_sai_runtime_resume+0x9c/0x1ec)^M
[] (fsl_sai_runtime_resume) from [] 
(__rpm_callback+0x3c/0x108)^M
[] (__rpm_callback) from [] (rpm_callback+0x60/0x64)^M
[] (rpm_callback) from [] (rpm_resume+0x5cc/0x808)^M
[] (rpm_resume) from [] (__pm_runtime_resume+0x60/0xa0)^M
[] (__pm_runtime_resume) from [] 
(fsl_sai_probe+0x2b8/0x65c)^M
[] (fsl_sai_probe) from [] (platform_probe+0x58/0xb8)^M
[] (platform_probe) from [] 
(really_probe.part.0+0x9c/0x334)^M
[] (really_probe.part.0) from [] 
(__driver_probe_device+0xa0/0x138)^M
[] (__driver_probe_device) from [] 
(driver_probe_device+0x30/0xc8)^M
[] (driver_probe_device) from [] 
(__driver_attach+0x90/0x130)^M
[] (__driver_attach) from [] (bus_for_each_dev+0x78/0xb8)^M
[] (bus_for_each_dev) from [] (bus_add_driver+0xf0/0x1d8)^M
[] (bus_add_driver) from [] (driver_register+0x88/0x118)^M
[] (driver_register) from [] (do_one_initcall+0x7c/0x3a4)^M
[] (do_one_initcall) from [] 
(kernel_init_freeable+0x198/0x22c)^M
[] (kernel_init_freeable) from [] (kernel_init+0x10/0x128)^M
[] (kernel_init) from [] (ret_from_fork+0x14/0x38)^M

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Guenter Roeck 
---
v2: Updated description to include tracebacks and to use the term 'instantiate'
Added inline comments describing devices in more detail
Added Philippe's Reviewed-by: tag

 hw/arm/fsl-imx6ul.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
index e0128d7316..1d1a708dd9 100644
--- a/hw/arm/fsl-imx6ul.c
+++ b/hw/arm/fsl-imx6ul.c
@@ -534,6 +534,13 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error 
**errp)
  */
 create_unimplemented_device("sdma", FSL_IMX6UL_SDMA_ADDR, 0x4000);
 
+/*
+ * SAI (Audio SSI (Synchronous Serial Interface))
+ */
+create_unimplemented_device("sai1", FSL_IMX6UL_SAI1_ADDR, 0x4000);
+create_unimplemented_device("sai2", FSL_IMX6UL_SAI2_ADDR, 0x4000);
+create_unimplemented_device("sai3", FSL_IMX6UL_SAI3_ADDR, 0x4000);
+
 /*
  * PWM
  */
@@ -542,6 +549,11 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error 
**errp)
 create_unimplemented_device("pwm3", FSL_IMX6UL_PWM3_ADDR, 0x4000);
 create_unimplemented_device("pwm4", FSL_IMX6UL_PWM4_ADDR, 0x4000);
 
+/*
+ * Audio ASRC (asynchronous sample rate converter)
+ */
+create_unimplemented_device("asrc", FSL_IMX6UL_ASRC_ADDR, 0x4000);
+
 /*
  * CAN
  */
-- 
2.25.1

Re: [PATCH v3 09/10] virtiofsd: Optionally fill lo_inode.fhandle

[Vivek Goyal]

On Tue, Aug 10, 2021 at 05:26:15PM +0200, Hanna Reitz wrote:
> On 10.08.21 17:23, Vivek Goyal wrote:
> > On Tue, Aug 10, 2021 at 10:32:55AM +0200, Hanna Reitz wrote:
> > > On 09.08.21 20:41, Vivek Goyal wrote:
> > > > On Fri, Jul 30, 2021 at 05:01:33PM +0200, Max Reitz wrote:
> > > > > When the inode_file_handles option is set, try to generate a file 
> > > > > handle
> > > > > for new inodes instead of opening an O_PATH FD.
> > > > > 
> > > > > Being able to open these again will require CAP_DAC_READ_SEARCH, so 
> > > > > the
> > > > > description text tells the user they will also need to specify
> > > > > -o modcaps=+dac_read_search.
> > > > > 
> > > > > Generating a file handle returns the mount ID it is valid for.  
> > > > > Opening
> > > > > it will require an FD instead.  We have mount_fds to map an ID to an 
> > > > > FD.
> > > > > get_file_handle() fills the hash map by opening the file we have
> > > > > generated a handle for.  To verify that the resulting FD indeed
> > > > > represents the handle's mount ID, we use statx().  Therefore, using 
> > > > > file
> > > > > handles requires statx() support.
> > > > So opening the file and storing that fd in mount_fds table might be
> > > > a potential problem with inotify work Ioannis is doing.
> > > > 
> > > > So say a file foo.txt was opened O_RDONLY and fd stored in mount_fs. Now
> > > > say user unlinks foo.txt. If notifications are enabled, final 
> > > > notification
> > > > will not be generated till this mount_fds fd is closed.
> > > > 
> > > > Now question is when will this fd be closed? If it closed at some
> > > > later point and then notification is generated, that will break
> > > > notificaitons.
> > > Currently, it is never closed.
> > > 
> > > > In fact even O_PATH fd is delaying notifications due to same reason.
> > > > But its not too bad as we close O_PATH fd pretty quickly after
> > > > unlinking. And we were hoping that file handle support will get rid
> > > > of this problem because we will not keep O_PATH fd open.
> > > > 
> > > > But, IIUC, mount_fds stuff will make it even worse. I did not see
> > > > the code which removes this fd from mount_fds. So I am not sure what's
> > > > the life time of this fd.
> > > The lifetime is forever.  If we wanted to remove it at some point, we’d 
> > > need
> > > to track how many file handles we have open for the given mount fd and 
> > > then
> > > remove it from the table once the count reaches 0, so it would still be
> > > delayed.
> > > 
> > > I think in practice the first thing that is looked up from some mount will
> > > probably be the root directory, which cannot be deleted before everything
> > > else on the mount is gone, so that would work.  We track how many handles
> > > are there, if the whole mount were to be deleted, I hope all lo_inodes are
> > > evicted, the count goes to 0, and we can drop the mount fd.
> > Keeping a reference count on mount_fd object make sense. So we probably
> > maintain this hash table and lookup using mount_id (as you are already
> > doing). All subsequent inodes from same filesystem will use same
> > object. Once all inodes have been flushed out, then mount_fd object
> > should go away as well (allowing for unmount on host).
> > 
> > > I think we can make the assumption that the mount fd is the root directory
> > > certain by, well, looking into mountinfo...  That would result in us 
> > > always
> > > opening the root node of the filesystem, so that first the whole 
> > > filesystem
> > > needs to disappear before it can be deleted (and our mount fd closed) –
> > > which should work, I guess?
> > This seems more reasonable. And I think that's what man page seems to
> > suggest.
> > 
> > The  mount_id  argument  returns an identifier for the filesystem 
> > mount
> > that corresponds to pathname.  This corresponds to the first  field 
> >  in
> > one  of  the  records in /proc/self/mountinfo.  Opening the 
> > pathname in
> > the fifth field of that record yields a file descriptor for  the  
> > mount
> > point;  that  file  descriptor  can  be  used  in  a subsequent 
> > call to
> > open_by_handle_at().
> > 
> > Fifth field seems to be the mount point. man proc says.
> > 
> >(5)  mount  point:  the  pathname of the mount point 
> > relative to
> > the process's root directory.
> > 
> > So opening mount point and saving as mount_fd (if it is not already
> > in hash table) and then take a per inode reference count on mount_fd
> > object looks like will solve the life time issue of mount_fd as
> > well as the issue of temporary failures arising because we can't
> > open a device special file.
> 
> Well, we’ve had this discussion before, and it’s possible that a filesystem
> has a device file as its mount point.

Yes. I think you did modified fuse to do some special trickery. Not sure
where should that be fixed. 

If filesystem is faking, then it can fake a device node

Re: [PATCH v2] MAINTAINERS: Name and email address change

[Peter Maydell]

On Tue, 10 Aug 2021 at 15:06, Hanna Reitz  wrote:
>
> I have changed my name and email address.  Update the MAINTAINERS file
> to match, and .mailmap in case anyone wants to send me an email because
> of some past commit I authored.  (As suggested by Philippe, I put the
> .mailmap line into the "preferred name forms" section, considering it
> counts as a git author config change.)
>
> Signed-off-by: Hanna Reitz 
> ---
> v2: .mailmap update
> ---
>  .mailmap| 1 +
>  MAINTAINERS | 8 
>  2 files changed, 5 insertions(+), 4 deletions(-)

Applied to master, thanks.

-- PMM

Re: [PULL ppc] powernv queue

[Peter Maydell]

On Tue, 10 Aug 2021 at 16:32, Philippe Mathieu-Daudé  wrote:
>
> On 8/10/21 5:17 PM, Peter Maydell wrote:
> > On Tue, 10 Aug 2021 at 13:46, Cédric Le Goater  wrote:
> >>
> >> The following changes since commit 
> >> bccabb3a5d60182645c7749e89f21a9ff307a9eb:
> >>
> >>   Update version for v6.1.0-rc2 release (2021-08-04 16:56:14 +0100)
> >>
> >> are available in the Git repository at:
> >>
> >>   https://github.com/legoater/qemu/ tags/pull-powernv-20210810
> >>
> >> for you to fetch changes up to 91a6b62df830d51f2b6b2ea00b3c92231d0ba9dc:
> >>
> >>   ppc/pnv: update skiboot to commit 820d43c0a775. (2021-08-10 14:18:51 
> >> +0200)
> >>
> >> 
> >> ppc/pnv: update skiboot image
> >>
> >> 
> >> Cédric Le Goater (1):
> >>   ppc/pnv: update skiboot to commit 820d43c0a775.
> >
> > That skiboot changelog lists a massive set of changes, which
> > is correspondingly a larger risk that something in there turns
> > out to be a must-fix-for-release regression.
>
> There might be a misunderstanding here, per
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg828857.html
> I understand this pull request is targetting David PPC tree,
> not the mainstream one. David first queued this patch for 6.2,
> and later confirmed by Cédric:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg829160.html

Mmm. Also as Dan pointed out on irc firmware updates ought to
be honouring the usual softfreeze rules. So I'm going to err
on the safe side and won't be applying this.

thanks
-- PMM

Re: [PULL ppc] powernv queue

[Philippe Mathieu-Daudé]

On 8/10/21 5:17 PM, Peter Maydell wrote:
> On Tue, 10 Aug 2021 at 13:46, Cédric Le Goater  wrote:
>>
>> The following changes since commit bccabb3a5d60182645c7749e89f21a9ff307a9eb:
>>
>>   Update version for v6.1.0-rc2 release (2021-08-04 16:56:14 +0100)
>>
>> are available in the Git repository at:
>>
>>   https://github.com/legoater/qemu/ tags/pull-powernv-20210810
>>
>> for you to fetch changes up to 91a6b62df830d51f2b6b2ea00b3c92231d0ba9dc:
>>
>>   ppc/pnv: update skiboot to commit 820d43c0a775. (2021-08-10 14:18:51 +0200)
>>
>> 
>> ppc/pnv: update skiboot image
>>
>> 
>> Cédric Le Goater (1):
>>   ppc/pnv: update skiboot to commit 820d43c0a775.
> 
> That skiboot changelog lists a massive set of changes, which
> is correspondingly a larger risk that something in there turns
> out to be a must-fix-for-release regression.

There might be a misunderstanding here, per
https://www.mail-archive.com/qemu-devel@nongnu.org/msg828857.html
I understand this pull request is targetting David PPC tree,
not the mainstream one. David first queued this patch for 6.2,
and later confirmed by Cédric:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg829160.html

> For future release cycles, can you try to get guest bios
> blob updates in earlier rather than on the day we're trying
> to get rc3 out, please?
> 
> thanks
> -- PMM
> 

Re: [PATCH v3 09/10] virtiofsd: Optionally fill lo_inode.fhandle

[Hanna Reitz]

On 10.08.21 17:23, Vivek Goyal wrote:

On Tue, Aug 10, 2021 at 10:32:55AM +0200, Hanna Reitz wrote:

On 09.08.21 20:41, Vivek Goyal wrote:

On Fri, Jul 30, 2021 at 05:01:33PM +0200, Max Reitz wrote:

When the inode_file_handles option is set, try to generate a file handle
for new inodes instead of opening an O_PATH FD.

Being able to open these again will require CAP_DAC_READ_SEARCH, so the
description text tells the user they will also need to specify
-o modcaps=+dac_read_search.

Generating a file handle returns the mount ID it is valid for.  Opening
it will require an FD instead.  We have mount_fds to map an ID to an FD.
get_file_handle() fills the hash map by opening the file we have
generated a handle for.  To verify that the resulting FD indeed
represents the handle's mount ID, we use statx().  Therefore, using file
handles requires statx() support.

So opening the file and storing that fd in mount_fds table might be
a potential problem with inotify work Ioannis is doing.

So say a file foo.txt was opened O_RDONLY and fd stored in mount_fs. Now
say user unlinks foo.txt. If notifications are enabled, final notification
will not be generated till this mount_fds fd is closed.

Now question is when will this fd be closed? If it closed at some
later point and then notification is generated, that will break
notificaitons.

Currently, it is never closed.


In fact even O_PATH fd is delaying notifications due to same reason.
But its not too bad as we close O_PATH fd pretty quickly after
unlinking. And we were hoping that file handle support will get rid
of this problem because we will not keep O_PATH fd open.

But, IIUC, mount_fds stuff will make it even worse. I did not see
the code which removes this fd from mount_fds. So I am not sure what's
the life time of this fd.

The lifetime is forever.  If we wanted to remove it at some point, we’d need
to track how many file handles we have open for the given mount fd and then
remove it from the table once the count reaches 0, so it would still be
delayed.

I think in practice the first thing that is looked up from some mount will
probably be the root directory, which cannot be deleted before everything
else on the mount is gone, so that would work.  We track how many handles
are there, if the whole mount were to be deleted, I hope all lo_inodes are
evicted, the count goes to 0, and we can drop the mount fd.

Keeping a reference count on mount_fd object make sense. So we probably
maintain this hash table and lookup using mount_id (as you are already
doing). All subsequent inodes from same filesystem will use same
object. Once all inodes have been flushed out, then mount_fd object
should go away as well (allowing for unmount on host).


I think we can make the assumption that the mount fd is the root directory
certain by, well, looking into mountinfo...  That would result in us always
opening the root node of the filesystem, so that first the whole filesystem
needs to disappear before it can be deleted (and our mount fd closed) –
which should work, I guess?

This seems more reasonable. And I think that's what man page seems to
suggest.

The  mount_id  argument  returns an identifier for the filesystem mount
that corresponds to pathname.  This corresponds to the first  field  in
one  of  the  records in /proc/self/mountinfo.  Opening the pathname in
the fifth field of that record yields a file descriptor for  the  mount
point;  that  file  descriptor  can  be  used  in  a subsequent call to
open_by_handle_at().

Fifth field seems to be the mount point. man proc says.

   (5)  mount  point:  the  pathname of the mount point relative to
the process's root directory.

So opening mount point and saving as mount_fd (if it is not already
in hash table) and then take a per inode reference count on mount_fd
object looks like will solve the life time issue of mount_fd as
well as the issue of temporary failures arising because we can't
open a device special file.


Well, we’ve had this discussion before, and it’s possible that a 
filesystem has a device file as its mount point.


But given the inotify complications, there’s really a good reason we 
should use mountinfo.



It’s a bit tricky because our sandboxing prevents easy access to mountinfo,
but if that’s the only way...

yes. We already have lo->proc_self_fd. Maybe we need to keep
/proc/self/mountinfo open in lo->proc_self_mountinfo. I am assuming
that any mount table changes will still be visible despite the fact
I have fd open (and don't have to open new fd to notice new mount/unmount
changes).


Well, yes, that was my idea.  Unfortunately, I wasn’t quite successful 
yet; when I tried keeping the fd open, reading from it would just return 
0 bytes.  Perhaps that’s because we bind-mount /proc/self/fd to /proc so 
that nothing else in /proc is visible. Perhaps we need to bind-mount 
/proc/self/mountinfo into /proc/self/fd before that...

Re: [PATCH v3 09/10] virtiofsd: Optionally fill lo_inode.fhandle

[Vivek Goyal]

On Tue, Aug 10, 2021 at 10:32:55AM +0200, Hanna Reitz wrote:
> On 09.08.21 20:41, Vivek Goyal wrote:
> > On Fri, Jul 30, 2021 at 05:01:33PM +0200, Max Reitz wrote:
> > > When the inode_file_handles option is set, try to generate a file handle
> > > for new inodes instead of opening an O_PATH FD.
> > > 
> > > Being able to open these again will require CAP_DAC_READ_SEARCH, so the
> > > description text tells the user they will also need to specify
> > > -o modcaps=+dac_read_search.
> > > 
> > > Generating a file handle returns the mount ID it is valid for.  Opening
> > > it will require an FD instead.  We have mount_fds to map an ID to an FD.
> > > get_file_handle() fills the hash map by opening the file we have
> > > generated a handle for.  To verify that the resulting FD indeed
> > > represents the handle's mount ID, we use statx().  Therefore, using file
> > > handles requires statx() support.
> > So opening the file and storing that fd in mount_fds table might be
> > a potential problem with inotify work Ioannis is doing.
> > 
> > So say a file foo.txt was opened O_RDONLY and fd stored in mount_fs. Now
> > say user unlinks foo.txt. If notifications are enabled, final notification
> > will not be generated till this mount_fds fd is closed.
> > 
> > Now question is when will this fd be closed? If it closed at some
> > later point and then notification is generated, that will break
> > notificaitons.
> 
> Currently, it is never closed.
> 
> > In fact even O_PATH fd is delaying notifications due to same reason.
> > But its not too bad as we close O_PATH fd pretty quickly after
> > unlinking. And we were hoping that file handle support will get rid
> > of this problem because we will not keep O_PATH fd open.
> > 
> > But, IIUC, mount_fds stuff will make it even worse. I did not see
> > the code which removes this fd from mount_fds. So I am not sure what's
> > the life time of this fd.
> 
> The lifetime is forever.  If we wanted to remove it at some point, we’d need
> to track how many file handles we have open for the given mount fd and then
> remove it from the table once the count reaches 0, so it would still be
> delayed.
> 
> I think in practice the first thing that is looked up from some mount will
> probably be the root directory, which cannot be deleted before everything
> else on the mount is gone, so that would work.  We track how many handles
> are there, if the whole mount were to be deleted, I hope all lo_inodes are
> evicted, the count goes to 0, and we can drop the mount fd.

Keeping a reference count on mount_fd object make sense. So we probably
maintain this hash table and lookup using mount_id (as you are already
doing). All subsequent inodes from same filesystem will use same
object. Once all inodes have been flushed out, then mount_fd object
should go away as well (allowing for unmount on host).

> 
> I think we can make the assumption that the mount fd is the root directory
> certain by, well, looking into mountinfo...  That would result in us always
> opening the root node of the filesystem, so that first the whole filesystem
> needs to disappear before it can be deleted (and our mount fd closed) –
> which should work, I guess?

This seems more reasonable. And I think that's what man page seems to 
suggest.

   The  mount_id  argument  returns an identifier for the filesystem mount
   that corresponds to pathname.  This corresponds to the first  field  in
   one  of  the  records in /proc/self/mountinfo.  Opening the pathname in
   the fifth field of that record yields a file descriptor for  the  mount
   point;  that  file  descriptor  can  be  used  in  a subsequent call to
   open_by_handle_at().

Fifth field seems to be the mount point. man proc says.

  (5)  mount  point:  the  pathname of the mount point relative to
   the process's root directory.

So opening mount point and saving as mount_fd (if it is not already
in hash table) and then take a per inode reference count on mount_fd
object looks like will solve the life time issue of mount_fd as
well as the issue of temporary failures arising because we can't
open a device special file.

> 
> It’s a bit tricky because our sandboxing prevents easy access to mountinfo,
> but if that’s the only way...

yes. We already have lo->proc_self_fd. Maybe we need to keep
/proc/self/mountinfo open in lo->proc_self_mountinfo. I am assuming
that any mount table changes will still be visible despite the fact
I have fd open (and don't have to open new fd to notice new mount/unmount
changes).

Vivek

Re: [PATCH v7 20/33] block/block-copy: make setting progress optional

[Hanna Reitz]

On 04.08.21 11:38, Vladimir Sementsov-Ogievskiy wrote:

Now block-copy will crash if user don't set progress meter by
block_copy_set_progress_meter(). copy-before-write filter will be used
in separate of backup job, and it doesn't want any progress meter (for
now). So, allow not setting it.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
---
  block/block-copy.c | 18 +++---
  1 file changed, 11 insertions(+), 7 deletions(-)


(Sorry for the accidental reply to v2...  I’ll just repeat myself:)

OK, looks a bit different from when I last reviewed this, because of the 
rebase on e3dd339feec2da3bcd82021e4ce4fe09dbf9c8b4, but R-b stands.


Hanna

Re: [PATCH v2 20/33] block/block-copy: make setting progress optional

[Hanna Reitz]

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:

Now block-copy will crash if user don't set progress meter by
block_copy_set_progress_meter(). copy-before-write filter will be used
in separate of backup job, and it doesn't want any progress meter (for
now). So, allow not setting it.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
---
  block/block-copy.c | 18 +++---
  1 file changed, 11 insertions(+), 7 deletions(-)


OK, looks a bit different because of the rebase on 
e3dd339feec2da3bcd82021e4ce4fe09dbf9c8b4, but R-b stands.


Hanna

Re: [PATCH] vl: fix machine option containing underscores

[Philippe Mathieu-Daudé]

+Paolo/Markus

On 8/10/21 3:12 PM, Jean-Philippe Brucker wrote:
> Since commit d8fb7d0969d5 ("vl: switch -M parsing to keyval"),
> keyval_dashify() replaces all underscores with dashes in machine
> options. As a result the machine option "default_bus_bypass_iommu",
> which was introduced in the same release (c9e96b04fc19 6d7a85483a06), is
> not recognized:
> 
> $ qemu-system-aarch64 -M virt,default_bus_bypass_iommu=on
> qemu-system-aarch64: Property 'virt-6.1-machine.default-bus-bypass-iommu' not 
> found
> 
> Before that commit, dashification was only applied temporarily within
> machine_set_property() to check the legacy options. Restore that
> behavior by explicitly checking for aliases of these options instead of
> transforming all machine options.
> 
> Fixes: d8fb7d0969d5 ("vl: switch -M parsing to keyval")
> Signed-off-by: Jean-Philippe Brucker 
> ---
> My first take was renaming default_bus_bypass_iommu, since it's the only
> machine option with underscores, but then we'd want to rename
> bypass_iommu as well for consistency and update all the docs. I prefer
> this but don't mind either way.
> ---
>  softmmu/vl.c | 56 +++-
>  1 file changed, 20 insertions(+), 36 deletions(-)
> 
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 5ca11e7469..3d3aa35279 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1660,41 +1660,25 @@ static int object_parse_property_opt(Object *obj,
>  return 0;
>  }
>  
> -/* *Non*recursively replace underscores with dashes in QDict keys.  */
> -static void keyval_dashify(QDict *qdict, Error **errp)
> +static const char *find_option_alias(QDict *qdict, const char *key,
> + const char *alias, const char **value)
>  {
> -const QDictEntry *ent, *next;
> -char *p;
> -
> -for (ent = qdict_first(qdict); ent; ent = next) {
> -g_autofree char *new_key = NULL;
> -
> -next = qdict_next(qdict, ent);
> -if (!strchr(ent->key, '_')) {
> -continue;
> -}
> -new_key = g_strdup(ent->key);
> -for (p = new_key; *p; p++) {
> -if (*p == '_') {
> -*p = '-';
> -}
> -}
> -if (qdict_haskey(qdict, new_key)) {
> -error_setg(errp, "Conflict between '%s' and '%s'", ent->key, 
> new_key);
> -return;
> -}
> -qobject_ref(ent->value);
> -qdict_put_obj(qdict, new_key, ent->value);
> -qdict_del(qdict, ent->key);
> +*value = qdict_get_try_str(qdict, key);
> +if (*value) {
> +return key;
> +}
> +*value = qdict_get_try_str(qdict, alias);
> +if (*value) {
> +return alias;
>  }
> +return NULL;
>  }
>  
>  static void qemu_apply_legacy_machine_options(QDict *qdict)
>  {
> +const char *key;
>  const char *value;
>  
> -keyval_dashify(qdict, &error_fatal);
> -
>  /* Legacy options do not correspond to MachineState properties.  */
>  value = qdict_get_try_str(qdict, "accel");
>  if (value) {
> @@ -1702,27 +1686,27 @@ static void qemu_apply_legacy_machine_options(QDict 
> *qdict)
>  qdict_del(qdict, "accel");
>  }
>  
> -value = qdict_get_try_str(qdict, "igd-passthru");
> -if (value) {
> +key = find_option_alias(qdict, "igd-passthru", "igd_passthru", &value);
> +if (key) {
>  object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), "igd-passthru", 
> value,
> false);
> -qdict_del(qdict, "igd-passthru");
> +qdict_del(qdict, key);
>  }
>  
> -value = qdict_get_try_str(qdict, "kvm-shadow-mem");
> -if (value) {
> +key = find_option_alias(qdict, "kvm-shadow-mem", "kvm_shadow_mem", 
> &value);
> +if (key) {
>  object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), 
> "kvm-shadow-mem", value,
> false);
> -qdict_del(qdict, "kvm-shadow-mem");
> +qdict_del(qdict, key);
>  }
>  
> -value = qdict_get_try_str(qdict, "kernel-irqchip");
> -if (value) {
> +key = find_option_alias(qdict, "kernel-irqchip", "kernel_irqchip", 
> &value);
> +if (key) {
>  object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), 
> "kernel-irqchip", value,
> false);
>  object_register_sugar_prop(ACCEL_CLASS_NAME("whpx"), 
> "kernel-irqchip", value,
> false);
> -qdict_del(qdict, "kernel-irqchip");
> +qdict_del(qdict, key);
>  }
>  }
>  
>