Re: [PATCH 00/55] qdev: Rework how we plug into the parent bus

[Paolo Bonzini]

All of it this time.

Paolo

Il mar 9 giu 2020, 08:41 Markus Armbruster  ha scritto:

> Paolo Bonzini  writes:
>
> > On 08/06/20 12:56, Markus Armbruster wrote:
> >>> Great stuff, I only had some comments on the commit messages.  I still
> >>> have to review patches 47 and 48 more corefully.
> >> Does this translate into any Reviewed-bys?  On v2, maybe?
> >>
> >
> > Yes, please add my Reviewed-by on v2.
>
> All of v2, or v2 less PATCH 49+50 (old 47+48)?
>
>

Re: [PATCH v2 1/1] virtio-ccw: auto-manage VIRTIO_F_IOMMU_PLATFORM if PV

[Cornelia Huck]

On Mon, 8 Jun 2020 19:00:45 +0200
Halil Pasic  wrote:


> > I'm also not 100% sure about migration... would it make sense to
> > discuss all of this in the context of the cross-arch patchset? It seems
> > power has similar issues.
> >   
> 
> I'm going to definitely have a good look at that. What I think special
> about s390 is that F_ACCESS_PLATFORM is hurting us because all IO needs
> to go through ZONE_DMA (this is a problem of the implementation that
> stemming form a limitation of the DMA API, upstream didn't let me
> fix it). 

My understanding is that power runs into similar issues, but I don't
know much about power, so I might be entirely wrong :)

> 
> > > 
> > > Further improvements are possible and probably necessary if we want
> > > to go down this path. But I would like to verify that first.
> > > 
> > > 8<-
> > > From: Halil Pasic 
> > > Date: Wed, 26 Feb 2020 16:48:21 +0100
> > > Subject: [PATCH v2.5 1/1] virtio-ccw: auto-manage VIRTIO_F_IOMMU_PLATFORM 
> > > if PV
> > > 
> > > The virtio specification tells that the device is to present
> > > VIRTIO_F_ACCESS_PLATFORM (a.k.a. VIRTIO_F_IOMMU_PLATFORM) when the
> > > device "can only access certain memory addresses with said access
> > > specified and/or granted by the platform". This is the case for a
> > > protected VMs, as the device can access only memory addresses that are
> > > in pages that are currently shared (only the guest can share/unsare its
> > > pages).
> > > 
> > > No VM, however, starts out as a protected VM, but some VMs may be
> > > converted to protected VMs if the guest decides so.
> > > 
> > > Making the end user explicitly manage the VIRTIO_F_ACCESS_PLATFORM via
> > > the property iommu_on is a minor disaster. Since the correctness of the
> > > paravirtualized virtio devices depends (and thus in a sense the
> > > correctness of the hypervisor) it, then the hypervisor should have the
> > > last word about whether VIRTIO_F_ACCESS_PLATFORM is to be presented or
> > > not.
> > > 
> > > Currently presenting a PV guest with a (paravirtualized) virtio-ccw
> > > device has catastrophic consequences for the VM (after the hypervisors
> > > access to protected memory). This is especially grave in case of device
> > > hotplug (because in this case the guest is more likely to be in the
> > > middle of something important).  
> > 
> > You mean for virtio-ccw devices that don't have iommu_on, right? 
> > 
> >   
> 
> Right, I'm missing the most important words.
> 
> > > 
> > > Let us add the ability to manage the VIRTIO_F_ACCESS_PLATFORM virtio
> > > feature automatically. This is accomplished  by turning the property
> > > into an 'on/off/auto' property, and for virtio-ccw devices if auto
> > > was specified forcing its value before  we start the protected VM. If
> > > the VM should cease to be protected, the original value is restored.
> > > 
> > > Signed-off-by: Halil Pasic 
> > > ---
> > >  hw/s390x/s390-virtio-ccw.c |  2 ++
> > >  hw/s390x/virtio-ccw.c  | 14 ++
> > >  hw/virtio/virtio.c | 19 +++
> > >  include/hw/virtio/virtio.h |  4 ++--
> > >  4 files changed, 37 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> > > index f660070d22..705e6b153a 100644
> > > --- a/hw/s390x/s390-virtio-ccw.c
> > > +++ b/hw/s390x/s390-virtio-ccw.c
> > > @@ -330,6 +330,7 @@ static void 
> > > s390_machine_unprotect(S390CcwMachineState *ms)
> > >  migrate_del_blocker(pv_mig_blocker);
> > >  error_free_or_abort(&pv_mig_blocker);
> > >  qemu_balloon_inhibit(false);
> > > +subsystem_reset();
> > >  }
> > >  
> > >  static int s390_machine_protect(S390CcwMachineState *ms)
> > > @@ -382,6 +383,7 @@ static int s390_machine_protect(S390CcwMachineState 
> > > *ms)
> > >  if (rc) {
> > >  goto out_err;
> > >  }
> > > +subsystem_reset();
> > >  return rc;
> > >  
> > >  out_err:
> > > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> > > index 64f928fc7d..2bb29b57aa 100644
> > > --- a/hw/s390x/virtio-ccw.c
> > > +++ b/hw/s390x/virtio-ccw.c
> > > @@ -874,6 +874,20 @@ static void virtio_ccw_reset(DeviceState *d)
> > >  VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
> > >  VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
> > >  VirtIOCCWDeviceClass *vdc = VIRTIO_CCW_DEVICE_GET_CLASS(dev);
> > > +S390CcwMachineState *ms = S390_CCW_MACHINE(qdev_get_machine());
> > > +
> > > +/*
> > > + * An attempt to use a paravirt device without 
> > > VIRTIO_F_IOMMU_PLATFORM
> > > + * in PV, has catastrophic consequences for the VM. Let's force
> > > + * VIRTIO_F_IOMMU_PLATFORM not already specified.
> > > + */
> > > +if (vdev->access_platform_auto && ms->pv) {
> > > +virtio_add_feature(&vdev->host_features, 
> > > VIRTIO_F_IOMMU_PLATFORM);
> > > +vdev->access_platform = ON_OFF_AUTO_ON;
> > > +} else if (

Re: [PATCH 00/55] qdev: Rework how we plug into the parent bus

[Markus Armbruster]

Paolo Bonzini  writes:

> On 08/06/20 12:56, Markus Armbruster wrote:
>>> Great stuff, I only had some comments on the commit messages.  I still
>>> have to review patches 47 and 48 more corefully.
>> Does this translate into any Reviewed-bys?  On v2, maybe?
>> 
>
> Yes, please add my Reviewed-by on v2.

All of v2, or v2 less PATCH 49+50 (old 47+48)?

Re: [PATCH v2 23/24] sd: Hide the qdev-but-not-quite thing created by sd_init()

[Markus Armbruster]

Philippe Mathieu-Daudé  writes:

> On 5/28/20 1:04 PM, Markus Armbruster wrote:
>> Commit 260bc9d8aa "hw/sd/sd.c: QOMify" QOMified only the device
>> itself, not its users.  It kept sd_init() around for non-QOMified
>> users.
>> 
>> More than four years later, three such users remain: omap1 (machines
>> cheetah, sx1, sx1-v1) and omap2 (machines n800, n810) are not
>> QOMified, and pl181 (machines integratorcp, realview-eb,
>> realview-eb-mpcore, realview-pb-a8 realview-pbx-a9, versatileab,
>> versatilepb, vexpress-a15, vexpress-a9) is not QOMified properly.
>> 
>> The issue I presently have with this: an "sd-card" device should plug
>> into an "sd-bus" (its DeviceClass member bus_type says so), but
>> sd_init() leaves it unplugged.  This is normally a bug (I just fixed
>> some instances), and I'd like to assert proper pluggedness to prevent
>> regressions.  However, the qdev-but-not-quite thing returned by
>> sd_init() would fail the assertion.  Meh.
>> 
>> Make sd_init() hide it from QOM/qdev.  Visible in "info qom-tree",
>> here's the change for cheetah:
>> 
>>  /machine (cheetah-machine)
>>[...]
>>/unattached (container)
>>  [...]
>>  /device[5] (serial-mm)
>>/serial (serial)
>>/serial[0] (qemu:memory-region)
>> -/device[6] (sd-card)
>> -/device[7] (omap-gpio)
>> +/device[6] (omap-gpio)
>>  [rest of device[*] renumbered...]
>> 
>> Cc: "Philippe Mathieu-Daudé" 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  hw/sd/sd.c | 40 
>>  1 file changed, 28 insertions(+), 12 deletions(-)
>> 
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 3c06a0ac6d..7070a116ea 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -83,6 +83,10 @@ enum SDCardStates {
>>  struct SDState {
>>  DeviceState parent_obj;
>>  
>> +/* If true, created by sd_init() for a non-qdevified caller */
>> +/* TODO purge them with fire */
>> +bool me_no_qdev_me_kill_mammoth_with_rocks;
>
> Your next patch does not use me_no_qdev_me_kill_mammoth_with_rocks in
> qdev_assert_realized_properly().

It doesn't have to, because this qdev-but-not-quite thing isn't visible
there.

> Suggestion for less ugly hack:
>
> static int qdev_assert_realized_properly(Object *obj, void *opaque)
> {
> DeviceState *dev = DEVICE(object_dynamic_cast(obj, TYPE_DEVICE));
> DeviceClass *dc;
>
> if (dev) {
> if (object_dynamic_cast(OBJECT(obj), TYPE_SD_CARD)) {
> /* bla bla bla */
> return 17;
> }
> dc = DEVICE_GET_CLASS(dev);
> assert(dev->realized);
> assert(dev->parent_bus || !dc->bus_type);
> }
> return 0;
> }

Now qdev_assert_realized_properly() knows about the caveman.  I don't
like that.

My hack keeps the knowledge strictly local, and protects all users of
QOM from getting exposed to the caveman, not just the "realized
properly" assertion.  My hack is locally ugly, but I consider that a
feature ;)

My patch could be made smaller: @me_no_qdev_me_kill_mammoth_with_rocks
exists only to make the parts supporting the caveman more immediately
obvious.

>
>> +
>>  /* SD Memory Card Registers */
>>  uint32_t ocr;
>>  uint8_t scr[8];
>> @@ -129,6 +133,8 @@ struct SDState {
>>  bool cmd_line;
>>  };
>>  
>> +static void sd_realize(DeviceState *dev, Error **errp);
>> +
>>  static const char *sd_state_name(enum SDCardStates state)
>>  {
>>  static const char *state_name[] = {
>> @@ -590,7 +596,7 @@ static void sd_cardchange(void *opaque, bool load, Error 
>> **errp)
>>  {
>>  SDState *sd = opaque;
>>  DeviceState *dev = DEVICE(sd);
>> -SDBus *sdbus = SD_BUS(qdev_get_parent_bus(dev));
>> +SDBus *sdbus;
>>  bool inserted = sd_get_inserted(sd);
>>  bool readonly = sd_get_readonly(sd);
>>  
>> @@ -601,19 +607,17 @@ static void sd_cardchange(void *opaque, bool load, 
>> Error **errp)
>>  trace_sdcard_ejected();
>>  }
>>  
>> -/* The IRQ notification is for legacy non-QOM SD controller devices;
>> - * QOMified controllers use the SDBus APIs.
>> - */
>> -if (sdbus) {
>> -sdbus_set_inserted(sdbus, inserted);
>> -if (inserted) {
>> -sdbus_set_readonly(sdbus, readonly);
>> -}
>> -} else {
>> +if (sd->me_no_qdev_me_kill_mammoth_with_rocks) {
>>  qemu_set_irq(sd->inserted_cb, inserted);
>>  if (inserted) {
>>  qemu_set_irq(sd->readonly_cb, readonly);
>>  }
>> +} else {
>> +sdbus = SD_BUS(qdev_get_parent_bus(dev));
>> +sdbus_set_inserted(sdbus, inserted);
>> +if (inserted) {
>> +sdbus_set_readonly(sdbus, readonly);
>> +}
>>  }
>>  }
>>  
>> @@ -697,6 +701,7 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
>>  {
>>  Object *obj;
>>  DeviceState *dev;
>> +SDState *sd;
>>  Error *err = NULL;
>>  
>>  obj = object_new(TYPE_SD_CARD);
>> @@ -707,13 

Re: [PATCH v2 3/8] MAINTAINERS: Mark SH4 TCG target orphan

[Aurelien Jarno]

On 2020-06-08 11:01, Philippe Mathieu-Daudé wrote:
> Aurelien Jarno expressed his desire to orphan the SH4 target [*]:
> 
>   I don't mind being [...] removed from there.
>   I do not really have time to work on that.
> 
> Mark the SH4 TCG target orphan.
> 
> Many thanks to Aurelien for his substantial contributions to QEMU,
> and for maintaining the SH4 TCG target for various years!
> 
> [*] https://www.mail-archive.com/qemu-devel@nongnu.org/msg708400.html
> 
> Message-Id: <20200601214125.ga1924...@aurel32.net>
> Acked-by: Aurelien Jarno 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  MAINTAINERS | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a012d9b74e..cd65bce72b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -296,8 +296,7 @@ F: tests/tcg/s390x/
>  L: qemu-s3...@nongnu.org
>  
>  SH4 TCG CPUs
> -M: Aurelien Jarno 
> -S: Odd Fixes
> +S: Orphan
>  F: target/sh4/
>  F: disas/sh4.c

Acked-by: Aurelien Jarno 

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [PATCH v2 10/24] macio: Delete unused "macio-gpio" devices

[Markus Armbruster]

Mark Cave-Ayland  writes:

> On 28/05/2020 12:04, Markus Armbruster wrote:
>
>> These devices go with the "via-pmu" device, which is controlled by
>> property "has-pmu".  macio_newworld_init() creates it unconditionally,
>> because the property has not been set then.  macio_newworld_realize()
>> realizes it only when the property is true.  Works, although it can
>> leave an unrealized device hanging around in the QOM composition tree.
>> Affects machine mac99 with via=cuda (default).
>> 
>> Delete the unused device by making macio_newworld_realize() unparent
>> it.  Visible in "info qom-tree":
>> 
>>  /machine (mac99-machine)
>>[...]
>>/unattached (container)
>>  /device[9] (macio-newworld)
>>[...]
>>/escc-legacy-port[8] (qemu:memory-region)
>>/escc-legacy-port[9] (qemu:memory-region)
>>/escc-legacy[0] (qemu:memory-region)
>> -  /gpio (macio-gpio)
>> -/gpio[0] (qemu:memory-region)
>>/ide[0] (macio-ide)
>>  /ide.0 (IDE)
>>  /pmac-ide[0] (qemu:memory-region)
>> 
>> Cc: Mark Cave-Ayland 
>> Cc: David Gibson 
>> Cc: qemu-...@nongnu.org
>> Signed-off-by: Markus Armbruster 
>> ---
>>  hw/misc/macio/macio.c | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
>> index 3779865ab2..b3dddf8be7 100644
>> --- a/hw/misc/macio/macio.c
>> +++ b/hw/misc/macio/macio.c
>> @@ -368,6 +368,8 @@ static void macio_newworld_realize(PCIDevice *d, Error 
>> **errp)
>>  memory_region_add_subregion(&s->bar, 0x16000,
>>  sysbus_mmio_get_region(sysbus_dev, 0));
>>  } else {
>> +object_unparent(OBJECT(&ns->gpio));
>> +
>>  /* CUDA */
>>  object_initialize_child(OBJECT(s), "cuda", &s->cuda, 
>> sizeof(s->cuda),
>>  TYPE_CUDA, &error_abort, NULL);
>
> This looks correct to me. Re-reading over the 3 different patch series you've 
> posted,
> I think it makes sense to merge them soon since even though there may be some 
> debate
> over init/realize in places, the benefits would far outweigh the drawbacks 
> IMO.

We can always improve on top.

Rebasing this stuff is quite time-consuming.

> Anyhow for this patch:
>
> Reviewed-by: Mark Cave-Ayland 

Thanks!

Re: [PATCH v2 02/24] display/xlnx_dp: Fix to realize "i2c-ddc" and "aux-to-i2c-bridge"

[Markus Armbruster]

Philippe Mathieu-Daudé  writes:

> On 5/28/20 1:04 PM, Markus Armbruster wrote:
>> xlnx_dp_init() creates these two devices, but they're never realized.
>> Affects machine xlnx-zcu102.
>> 
>> In theory, a device becomes real only on realize.  In practice, the
>> transition from unreal to real is a fuzzy one.  The work to make a
>> device real can be spread between realize methods (fine),
>> instance_init methods (wrong), and board code wiring up the device
>> (fine as long as it effectively happens on realize).  Depending on
>> what exactly is done where, a device can work even when we neglect to
>> realize it.
>> 
>> These two appear to work.  Nevertheless, it's a clear misuse of the
>> interface.  Even when it works today (more or less by chance), it can
>> break tomorrow.
>> 
>> Fix by realizing them in xlnx_dp_realize().
>> 
>> Fixes: 58ac482a66de09a7590f705e53fc6a3fb8a055e8
>> Cc: KONRAD Frederic 
>> Cc: Alistair Francis 
>> Cc: "Edgar E. Iglesias" 
>> Cc: Peter Maydell 
>> Cc: qemu-...@nongnu.org
>> Signed-off-by: Markus Armbruster 
>> Reviewed-by: Edgar E. Iglesias 
>> Reviewed-by: Alistair Francis 
>> ---
>>  hw/display/xlnx_dp.c | 4 
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
>> index 3e5fb44e06..bdc229a51e 100644
>> --- a/hw/display/xlnx_dp.c
>> +++ b/hw/display/xlnx_dp.c
>> @@ -1264,9 +1264,13 @@ static void xlnx_dp_realize(DeviceState *dev, Error 
>> **errp)
>>  DisplaySurface *surface;
>>  struct audsettings as;
>>  
>> +qdev_init_nofail(DEVICE(s->aux_bus->bridge));
>
> Eh??? Why not realize the bridge in aux_init_bus()?

Because then aux_init_bus() is no longer "init", but "init and realize".
Instead: "[PATCH v2 32/58] auxbus: New aux_bus_realize(), pairing with
aux_bus_init()".  Okay?

>> +
>>  qdev_init_nofail(DEVICE(s->dpcd));
>>  aux_map_slave(AUX_SLAVE(s->dpcd), 0x);
>>  
>> +qdev_init_nofail(DEVICE(s->edid));
>
> This one is OK.
>
>> +
>>  s->console = graphic_console_init(dev, 0, &xlnx_dp_gfx_ops, s);
>>  surface = qemu_console_surface(s->console);
>>  xlnx_dpdma_set_host_data_location(s->dpdma, DP_GRAPHIC_DMA_CHANNEL,
>> 

Re: [PATCH v2 21/24] sparc/leon3: Fix to put grlib,* devices on sysbus

[Philippe Mathieu-Daudé]

On 5/28/20 1:04 PM, Markus Armbruster wrote:
> leon3_generic_hw_init() creates a "grlib,ahbpnp" and a "grlib,apbpnp"
> sysbus device in a way that leaves them unplugged.
> 
> Create them the common way that puts them into the main system bus.
> Affects machine leon3_generic.  Visible in "info qtree":
> 
>  bus: main-system-bus
>type System
> +  dev: grlib,ahbpnp, id ""
> +mmio f000/1000
> +  dev: grlib,apbpnp, id ""
> +mmio 800ff000/1000
>dev: grlib,irqmp, id ""
> 
> Cc: Fabien Chouteau 
> Cc: KONRAD Frederic 
> Cc: Mark Cave-Ayland 
> Cc: Artyom Tarasenko 
> Signed-off-by: Markus Armbruster 
> Reviewed-by: KONRAD Frederic 
> Reviewed-by: Philippe Mathieu-Daudé 
> ---
>  hw/sparc/leon3.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
> index 8f024dab7b..3facb8c2ae 100644
> --- a/hw/sparc/leon3.c
> +++ b/hw/sparc/leon3.c
> @@ -213,14 +213,14 @@ static void leon3_generic_hw_init(MachineState *machine)
>  reset_info->sp= LEON3_RAM_OFFSET + ram_size;
>  qemu_register_reset(main_cpu_reset, reset_info);
>  
> -ahb_pnp = GRLIB_AHB_PNP(object_new(TYPE_GRLIB_AHB_PNP));
> +ahb_pnp = GRLIB_AHB_PNP(qdev_create(NULL, TYPE_GRLIB_AHB_PNP));
>  object_property_set_bool(OBJECT(ahb_pnp), true, "realized", 
> &error_fatal);
>  sysbus_mmio_map(SYS_BUS_DEVICE(ahb_pnp), 0, LEON3_AHB_PNP_OFFSET);
>  grlib_ahb_pnp_add_entry(ahb_pnp, 0, 0, GRLIB_VENDOR_GAISLER,
>  GRLIB_LEON3_DEV, GRLIB_AHB_MASTER,
>  GRLIB_CPU_AREA);
>  
> -apb_pnp = GRLIB_APB_PNP(object_new(TYPE_GRLIB_APB_PNP));
> +apb_pnp = GRLIB_APB_PNP(qdev_create(NULL, TYPE_GRLIB_APB_PNP));
>  object_property_set_bool(OBJECT(apb_pnp), true, "realized", 
> &error_fatal);
>  sysbus_mmio_map(SYS_BUS_DEVICE(apb_pnp), 0, LEON3_APB_PNP_OFFSET);
>  grlib_ahb_pnp_add_entry(ahb_pnp, LEON3_APB_PNP_OFFSET, 0xFFF,
> 

Thanks, patch applied to for the next (temporary) sparc-next pull request.

Re: [PATCH 0/7] hw/sparc/leon3: Few fixes and disable HelenOS test

[Philippe Mathieu-Daudé]

On 3/31/20 12:50 PM, Philippe Mathieu-Daudé wrote:
> The Leon3Machine.test_leon3_helenos_uimage has been running
> erratically since some time now. Time to disable it (at least
> until we make it reliable).
> Few other patches added while tracking the issue.
> 
> Philippe Mathieu-Daudé (7):
>   tests/acceptance/machine_sparc_leon3: Disable HelenOS test
>   hw/misc/grlib_ahb_apb_pnp: Avoid crash when writing to AHB PnP
> registers
>   hw/misc/grlib_ahb_apb_pnp: Fix AHB PnP 8-bit accesses
>   hw/misc/grlib_ahb_apb_pnp: Add trace events on read accesses
>   hw/timer/grlib_gptimer: Display frequency in decimal
>   target/sparc/int32_helper: Remove DEBUG_PCALL definition
>   target/sparc/int32_helper: Extract and use excp_name_str()
> 
>  hw/misc/grlib_ahb_apb_pnp.c | 24 ++--
>  target/sparc/int32_helper.c | 23 ---
>  hw/misc/trace-events|  4 
>  hw/timer/trace-events   |  2 +-
>  tests/acceptance/machine_sparc_leon3.py |  4 
>  5 files changed, 43 insertions(+), 14 deletions(-)
> 

Thanks, series applied to for the next (temporary) sparc-next pull request.

Re: [PATCH 0/3] hw/sparc: Map the UART devices unconditionally

[Philippe Mathieu-Daudé]

On 6/8/20 7:21 PM, Philippe Mathieu-Daudé wrote:
> Few more SPARC patches.
> 
> Mark/Artyom/Frederic if you Ack them I'll simply add them
> to the current trivial SPARC patch queue I prepared.
> 
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (3):
>   hw/sparc/leon3: Map the UART device unconditionally
>   hw/sparc64/niagara: Remove duplicated NIAGARA_UART_BASE definition
>   hw/sparc64/niagara: Map the UART device unconditionally
> 
>  hw/sparc/leon3.c | 18 --
>  hw/sparc64/niagara.c |  7 ++-
>  2 files changed, 10 insertions(+), 15 deletions(-)
> 

Thanks, series applied to for the next (temporary) sparc-next pull request.

Re: [PATCH 0/7] hw/misc/empty_slot: Spring cleaning

[Philippe Mathieu-Daudé]

On 5/10/20 5:28 PM, Philippe Mathieu-Daudé wrote:
> New Spring, new opportunity to clean this device :)
> (v1 was in 2018, v2 in 2019).
> 
> - lower device priority
> - follow qdev model and use properties
> - convert to trace events
> - describe with slot name
> - move under hw/misc/ and cover in MAINTAINERS
> 
> Peter, I hope you are OK adding it wit UNIMP device,
> as both are very similar, and don't have much activity.
> 
> Only MIPS/SPARC32 targets use this device.
> 
> v2: https://www.mail-archive.com/qemu-devel@nongnu.org/msg626498.html
> v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg564060.html
> 
> Philippe Mathieu-Daudé (7):
>   hw/sparc/sun4m: Use UnimplementedDevice for I/O devices
>   hw/misc/empty_slot: Lower address space priority
>   hw/misc/empty_slot: Convert 'size' field as qdev property
>   hw/misc/empty_slot: Add a 'name' qdev property
>   hw/misc/empty_slot: Convert debug printf() to trace event
>   hw/misc/empty_slot: Move the 'hw/misc' and cover in MAINTAINERS
>   hw/misc/empty_slot: Name the slots when created
> 
>  include/hw/empty_slot.h|  9 ---
>  include/hw/misc/empty_slot.h   | 19 ++
>  hw/mips/mips_malta.c   |  4 +--
>  hw/{core => misc}/empty_slot.c | 47 +++---
>  hw/sparc/sun4m.c   | 23 +++--
>  MAINTAINERS|  4 ++-
>  hw/core/Makefile.objs  |  1 -
>  hw/misc/Makefile.objs  |  1 +
>  hw/misc/trace-events   |  4 +++
>  9 files changed, 70 insertions(+), 42 deletions(-)
>  delete mode 100644 include/hw/empty_slot.h
>  create mode 100644 include/hw/misc/empty_slot.h
>  rename hw/{core => misc}/empty_slot.c (66%)
> 

Thanks - except the MAINTAINERS change (merging empty_slot with
unimp device) which Peter did not ack - series applied to for
the next (temporary) sparc-next pull request.

Re: [PATCH] Makefile: Install qemu-[qmp/ga]-ref.* into the directory "interop"

[Philippe Mathieu-Daudé]

On 6/9/20 2:47 AM, Yi Wang wrote:
> From: Liao Pingfang 
> 
> We need install qemu-[qmp/ga]-ref.* files into the subdirectory of qemu docs: 
> interop.
> 
> If we visit the following address and click the link to qemu-qmp-ref.html:
> https://www.qemu.org/docs/master/interop/bitmaps.html#basic-qmp-usage
> 
> It will report following error:
> "
> Not Found
> The requested URL /docs/master/interop/qemu-qmp-ref.html was not found on 
> this server.
> "
> 

Fixes: d59157ea058b5 ('docs: create interop/ subdirectory')
Reviewed-by: Philippe Mathieu-Daudé 

> Signed-off-by: Liao Pingfang 
> ---
>  Makefile   | 10 ++
>  docs/index.html.in |  4 ++--
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 40e4f76..49dbe7a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -879,8 +879,9 @@ install-sphinxdocs: sphinxdocs
>  install-doc: $(DOCS) install-sphinxdocs
>   $(INSTALL_DIR) "$(DESTDIR)$(qemu_docdir)"
>   $(INSTALL_DATA) $(MANUAL_BUILDDIR)/index.html "$(DESTDIR)$(qemu_docdir)"
> - $(INSTALL_DATA) docs/interop/qemu-qmp-ref.html 
> "$(DESTDIR)$(qemu_docdir)"
> - $(INSTALL_DATA) docs/interop/qemu-qmp-ref.txt "$(DESTDIR)$(qemu_docdir)"
> + $(INSTALL_DIR) "$(DESTDIR)$(qemu_docdir)/interop"
> + $(INSTALL_DATA) docs/interop/qemu-qmp-ref.html 
> "$(DESTDIR)$(qemu_docdir)/interop"
> + $(INSTALL_DATA) docs/interop/qemu-qmp-ref.txt 
> "$(DESTDIR)$(qemu_docdir)/interop"
>  ifdef CONFIG_POSIX
>   $(INSTALL_DIR) "$(DESTDIR)$(mandir)/man1"
>   $(INSTALL_DATA) $(MANUAL_BUILDDIR)/system/qemu.1 
> "$(DESTDIR)$(mandir)/man1"
> @@ -898,8 +899,9 @@ ifdef CONFIG_TRACE_SYSTEMTAP
>  endif
>  ifneq (,$(findstring qemu-ga,$(TOOLS)))
>   $(INSTALL_DATA) $(MANUAL_BUILDDIR)/interop/qemu-ga.8 
> "$(DESTDIR)$(mandir)/man8"
> - $(INSTALL_DATA) docs/interop/qemu-ga-ref.html "$(DESTDIR)$(qemu_docdir)"
> - $(INSTALL_DATA) docs/interop/qemu-ga-ref.txt "$(DESTDIR)$(qemu_docdir)"
> + $(INSTALL_DIR) "$(DESTDIR)$(qemu_docdir)/interop"
> + $(INSTALL_DATA) docs/interop/qemu-ga-ref.html 
> "$(DESTDIR)$(qemu_docdir)/interop"
> + $(INSTALL_DATA) docs/interop/qemu-ga-ref.txt 
> "$(DESTDIR)$(qemu_docdir)/interop"
>   $(INSTALL_DATA) docs/interop/qemu-ga-ref.7 "$(DESTDIR)$(mandir)/man7"
>  endif
>  endif
> diff --git a/docs/index.html.in b/docs/index.html.in
> index e9a1603..6736fa4 100644
> --- a/docs/index.html.in
> +++ b/docs/index.html.in
> @@ -12,8 +12,8 @@
>  Tools Guide
>  System Emulation Management and 
> Interoperability Guide
>  System Emulation Guest Hardware 
> Specifications
> -QMP Reference Manual
> -Guest Agent Protocol 
> Reference
> +QMP Reference 
> Manual
> +Guest Agent Protocol 
> Reference
>  
>  
>  
> 

Re: [PATCH v1 7/9] plugins: add API to return a name for a IO device

[Emilio G. Cota]

On Mon, Jun 08, 2020 at 09:06:17 +0100, Alex Bennée wrote:
> Emilio G. Cota  writes:
> > I'd rather use asprintf(3) and strdup(3) here, so that plugins don't
> > have to worry about glib, and on the QEMU side we don't have to worry
> > about plugins calling free() instead of g_free().
> 
> AFAIK you can actually mix free/g_free because g_free is just a NULL
> checking wrapper around free.

I was just going with the documentation, but you're right:

https://github.com/GNOME/glib/blob/mainline/glib/gmem.c#L196
> void
> g_free (gpointer mem)
> {
>   free (mem);
>   TRACE(GLIB_MEM_FREE((void*) mem));
> }

The NULL-pointer check is done by free(3), though.

> However ideally I'd be passing a
> non-freeable const char to the plugin but I didn't want to expose
> pointers deep inside of QEMU's guts although maybe I'm just being
> paranoid there given you can easily gdb the combined operation anyway.
>
> Perhaps there is a need for a separate memory region where we can store
> copies of strings we have made for the plugins?

I agree with the idea of not exposing internal pointers to plugins
(e.g. we don't pass a CPUState *, only an opaque handle) so I'm OK
with returning a dup'ed string here.

(snip)
> That said in another
> thread Peter was uncomfortable about exposing this piece of information
> to plugins. Maybe we should only expose something based on the optional
> -device foo,id=bar parameter?

I have no opinion on whether exposing this is a good idea. If it turns
out that it is, please have my

Reviewed-by: Emilio G. Cota 

Thanks,

Emilio

Re: [PATCH] target/mips: Fix PageMask with variable page size

[Jiaxun Yang]

在 2020/6/9 10:47, Jiaxun Yang 写道:

Our current code assumed the target page size is always 4k
when handling PageMask and VPN2, however, variable page size
was just added to mips target and that's nolonger true.

So we refined this piece of code to handle any target page size.
Also added Big Page support defined by MIPS64 Release2.

Signed-off-by: Jiaxun Yang 
---


Sorry, this version may break MIPS32 build.

I'll send v2 very soon.

Thanks.

[...]

--
- Jiaxun

Re: [RFC v3 7/8] vhost-vdpa: introduce vhost-vdpa backend

[Cindy Lu]

On Tue, Jun 9, 2020 at 4:14 AM Eric Blake  wrote:
>
> On 5/29/20 9:06 AM, Cindy Lu wrote:
> > From: Tiwei Bie 
>
> The original author is Tiwei Bie...
>
> >
> > Currently we have 2 types of vhost backends in QEMU: vhost kernel and
> > vhost-user. The above patch provides a generic device for vDPA purpose,
> > this vDPA device exposes to user space a non-vendor-specific configuration
> > interface for setting up a vhost HW accelerator, this patch set introduces
> > a third vhost backend called vhost-vdpa based on the vDPA interface.
> >
> > Vhost-vdpa usage:
> >
> >qemu-system-x86_64 -cpu host -enable-kvm \
> >  ..
> >-netdev type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-id,id=vhost-vdpa0 \
> >-device virtio-net-pci,netdev=vhost-vdpa0,page-per-vq=on \
> >
> > Co-Authored-By: Lingshan zhu 
> > Signed-off-by: Cindy Lu 
> > ---
>
> ...but there is no S-o-b here.  Also, Co-Authored-By is an unusual tag;
> it's just as easy to spell it Signed-off-by even for co-authors.
>
> [Pardon my delicacy in wording my response; I unfortunately lack enough
> cultural context to know a preferred name or even gender-correct
> pronouns for referring to the authors in shorthand]
>
Thanks Eric for pointing that out :-), I will fix this soon.

Thanks
Cindy

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

Re: fpu/softfloat: a question on BFloat 16 support on QEMU

[LIU Zhiwei]

On 2020/6/9 3:34, Richard Henderson wrote:

On 6/8/20 5:53 AM, LIU Zhiwei wrote:

Hi Richard,

I am doing bfloat16 support on QEMU.

Once I tried to reuse float32 interface, but I couldn't properly process
rounding in some insns like fadd.

What's your opinion about it? Should I expand the fpu/softfloat?

Yes, we need to expand fpu/softfloat.

You'll want something like

static const FloatFmt bfloat16_params = {
 FLOAT_PARAMS(8, 7)
};

(This would be the Arm and x86 definition, anyway; hopefully risc-v is the 
same.)

Yes. It's the same for me.

And then add all of the other interface functions that you need to use that
parameter.

FWIW, it appears that Arm only requires:

   float32_to_bfloat16
   bfloat16_mul
   bfloat16_add

and I could even get away with only float32_to_bfloat16, since technically
Arm's BFAdd and BFMul psuedo-code are implemented in terms of single-precision
arithmetic, followed by a round-to-odd into BFloat16.

Some different here.

It's directly implemented in half-precision unit. So I'm afraid I have 
to implement as many as interfaces like float16.


Best Regards,
Zhiwei




r~

Re: fpu/softfloat: a question on BFloat 16 support on QEMU

[LIU Zhiwei]

On 2020/6/8 23:50, Alex Bennée wrote:

LIU Zhiwei  writes:


Hi Richard,

I am doing bfloat16 support on QEMU.

Once I tried to reuse float32 interface, but I couldn't properly process
rounding in some insns like fadd.

What do you mean by re-use the float32 interface?

Once I think bfloat16 can been converted to float32  by

deposit32(0, 16, 16, bf16)

Then do a bfloat16 op by float32 op.

At last, get the bfloat16 result by right shift the float32 result 16 bits.

Isn't bfloat16 going
to be pretty much the same as float16 but with some slightly different
float parameters for the different encoding?

Agree.

Like the float16 code it won't have to deal with any of the hardfloat
wrappers so it should look pretty similar.
Good idea. I will list the float16 interfaces,  and try to emulate the 
bfloat16 one by one.


I list float16 interfaces in softfloat.c alone. It counts 67 interfaces.

What's your opinion about it? Should I expand the fpu/softfloat?

bfloat16 is certainly going to become more common that we should have
common softfloat code to handle it. It would be nice is TestFloat could
exercise it as well.

Thanks. I will try to use make check-softfloat to test bfloat16 interfaces.

Best Regards,
Zhiwei

Best Regards,
Zhiwei

[PATCH] target/mips: Fix PageMask with variable page size

[Jiaxun Yang]

Our current code assumed the target page size is always 4k
when handling PageMask and VPN2, however, variable page size
was just added to mips target and that's nolonger true.

So we refined this piece of code to handle any target page size.
Also added Big Page support defined by MIPS64 Release2.

Signed-off-by: Jiaxun Yang 
---
 target/mips/cp0_helper.c | 48 ++--
 target/mips/cpu.h|  3 ++-
 2 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/target/mips/cp0_helper.c b/target/mips/cp0_helper.c
index bbf12e4a97..7a134085f7 100644
--- a/target/mips/cp0_helper.c
+++ b/target/mips/cp0_helper.c
@@ -872,20 +872,44 @@ void helper_mtc0_memorymapid(CPUMIPSState *env, 
target_ulong arg1)
 }
 }
 
-void update_pagemask(CPUMIPSState *env, target_ulong arg1, int32_t *pagemask)
+void helper_mtc0_pagemask(CPUMIPSState *env, target_ulong arg1)
 {
-uint64_t mask = arg1 >> (TARGET_PAGE_BITS + 1);
-if (!(env->insn_flags & ISA_MIPS32R6) || (arg1 == ~0) ||
-(mask == 0x || mask == 0x0003 || mask == 0x000F ||
- mask == 0x003F || mask == 0x00FF || mask == 0x03FF ||
- mask == 0x0FFF || mask == 0x3FFF || mask == 0x)) {
-env->CP0_PageMask = arg1 & (0x1FFF & (TARGET_PAGE_MASK << 1));
+uint64_t mask;
+int maxmaskbits, maskbits;
+
+if (env->insn_flags & ISA_MIPS32R6) {
+return;
 }
-}
 
-void helper_mtc0_pagemask(CPUMIPSState *env, target_ulong arg1)
-{
-update_pagemask(env, arg1, &env->CP0_PageMask);
+/* Don't care MASKX as we don't support 1KB page */
+#ifdef TARGET_MIPS64
+if (env->CP0_Config3 & CP0C3_BPG) {
+maxmaskbits = 47;
+} else {
+maxmaskbits = 16;
+}
+#else
+maxmaskbits = 16;
+#endif
+mask = extract64((uint64_t)arg1, CP0PM_MASK, maxmaskbits);
+
+maskbits = find_first_zero_bit(&mask, 64);
+
+/* Ensure no more set bit after first zero */
+if (mask >> maskbits) {
+goto invalid;
+}
+/* We don't support VTLB entry smaller than target page */
+if ((maskbits + 12) < TARGET_PAGE_BITS) {
+goto invalid;
+}
+env->CP0_PageMask = mask << CP0PM_MASK;
+
+return;
+
+invalid:
+maskbits = MIN(maxmaskbits, MAX(maskbits, TARGET_PAGE_BITS - 12));
+env->CP0_PageMask = ((1 << (maskbits + 1)) - 1) << CP0PM_MASK;
 }
 
 void helper_mtc0_pagegrain(CPUMIPSState *env, target_ulong arg1)
@@ -,7 +1135,7 @@ void helper_mthc0_saar(CPUMIPSState *env, target_ulong 
arg1)
 void helper_mtc0_entryhi(CPUMIPSState *env, target_ulong arg1)
 {
 target_ulong old, val, mask;
-mask = (TARGET_PAGE_MASK << 1) | env->CP0_EntryHi_ASID_mask;
+mask = ~((1 << 14) - 1) | env->CP0_EntryHi_ASID_mask;
 if (((env->CP0_Config4 >> CP0C4_IE) & 0x3) >= 2) {
 mask |= 1 << CP0EnHi_EHINV;
 }
diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 0b3c987bb3..b69806792d 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -617,7 +617,8 @@ struct CPUMIPSState {
 /*
  * CP0 Register 5
  */
-int32_t CP0_PageMask;
+target_ulong CP0_PageMask;
+#define CP0PM_MASK 13
 int32_t CP0_PageGrain_rw_bitmask;
 int32_t CP0_PageGrain;
 #define CP0PG_RIE 31
-- 
2.27.0.rc2

Re: [PATCH 5/6] hw/arm: Add NPCM730 and NPCM750 SoC models

[Joel Stanley]

On Thu, 21 May 2020 at 20:39, Havard Skinnemoen  wrote:
>
> The Nuvoton NPCM7xx SoC family are used to implement Baseboard
> Management Controllers in servers. While the family includes four SoCs,
> this patch implements limited support for two of them: NPCM730 (targeted
> for Data Center applications) and NPCM750 (targeted for Enterprise
> applications).
>
> This patch includes little more than the bare minimum needed to boot a
> Linux kernel built with NPCM7xx support in direct-kernel mode:
>
>   - Two Cortex-A9 CPU cores with built-in periperhals.
>   - Global Configuration Registers.
>   - Clock Management.
>   - 3 Timer Modules with 5 timers each.
>   - 4 serial ports.
>
> The chips themselves have a lot more features, some of which will be
> added to the model at a later stage.
>
> Reviewed-by: Tyrone Ting 
> Signed-off-by: Havard Skinnemoen 

This looks okay from my perspective. You will want to get Cedric or
Peter to take a closer look though.

We have started adding about the systems that are supported by qemu,
including how to boot the machine, what is supported and the
limitations of the modelling. See docs/system/arm/aspeed.rst for an
example.

Reviewed-by: Joel Stanley 

Cheers,

Joel

Re: [PATCH 0/6] Add Nuvoton NPCM730/NPCM750 SoCs and two BMC machines

[Joel Stanley]

On Mon, 8 Jun 2020 at 22:15, Havard Skinnemoen  wrote:
>
> On Thu, May 21, 2020 at 12:21 PM Havard Skinnemoen  
> wrote:
>>
>> This patch series models enough of the Nuvoton NPCM730 and NPCM750 SoCs to 
>> boot
>> a minimal Linux kernel. This includes device models for:
>
>
> Does anyone have comments on this series? I'm currently finishing up a second 
> patch series that adds flash support and a few other things so qemu can boot 
> a full OpenBMC flash image built for npcm7xx.

I had a look and they appear good to me. Note that I'm less in to the
gory details of Qemu than some of our other reviewers, so you should
seek  a more detailed review from someone else.

I look forward further support so I can test the OpenBMC kernel
against Nuvoton boards in the same way as the Aspeed ones.

Cheers,

Joel

>
> If you prefer, I can combine them both into one series and send it to the 
> list.
>
> This series can be found here: 
> https://patchwork.kernel.org/project/qemu-devel/list/?series=291809
>
> Thanks,
>
> Havard

Re: [PATCH 2/6] hw/misc: Add NPCM7xx System Global Control Registers device model

[Joel Stanley]

On Thu, 21 May 2020 at 20:40, Havard Skinnemoen  wrote:
>
> Implement a device model for the System Global Control Registers in the
> NPCM730 and NPCM750 BMC SoCs.
>
> This is primarily used to enable SMP boot (the boot ROM spins reading
> the SCRPAD register); other registers are best effort for now.
>
> The reset values of the MDLR and PWRON registers are determined by the
> SoC variant (730 vs 750) and board straps respectively.
>
> Reviewed-by: Tyrone Ting 
> Signed-off-by: Havard Skinnemoen 

This looks similar to the aspeed scu model :)

Reviewed-by: Joel Stanley 

Re: [PATCH 6/6] hw/arm: Add two NPCM7xx-based machines

[Joel Stanley]

On Thu, 21 May 2020 at 20:39, Havard Skinnemoen  wrote:
>
> This adds two new machines:
>
>   - npcm750-evb: Nuvoton NPCM750 Evaluation Board.
>   - quanta-gsj: A board with a NPCM730 chip.

You could note that these are the two boards supported by OpenBMC.

> They rely on the NPCM7xx SoC device to do the heavy lifting. They are
> almost completely identical at the moment, apart from the SoC type,
> which currently only changes the reset contents of one register
> (GCR.MDLR), but they might grow apart a bit more as more functionality
> is added.
>
> Both machines can boot the Linux kernel into /bin/sh.
>
> Reviewed-by: Tyrone Ting 
> Signed-off-by: Havard Skinnemoen 

Reviewed-by: Joel Stanley 

Re: [PATCH 3/6] hw/misc: Add NPCM7xx Clock Controller device model

[Joel Stanley]

On Thu, 21 May 2020 at 20:39, Havard Skinnemoen  wrote:
>
> Enough functionality to boot the Linux kernel has been implemented. This
> includes:
>
>   - Correct power-on reset values so the various clock rates can be
> accurately calculated.
>   - Clock enables stick around when written.
>
> In addition, a best effort attempt to implement SECCNT and CNTR25M was
> made even though I don't think the kernel needs them.
>
> Reviewed-by: Tyrone Ting 
> Signed-off-by: Havard Skinnemoen 

Reviewed-by: Joel Stanley 

> +++ b/hw/misc/npcm7xx_clk.c

> +#define PLLCON_LOKI BIT(31)
> +#define PLLCON_LOKS BIT(30)
> +#define PLLCON_PWDENBIT(12)
> +
> +static const uint32_t cold_reset_values[NPCM7XX_CLK_NR_REGS] = {

Can you add a comment to mention where these come from? If it's the
data sheet, note which version you're using.


> +[NPCM7XX_CLK_CLKEN1]= 0x,
> +[NPCM7XX_CLK_CLKSEL]= 0x004a,
> +[NPCM7XX_CLK_CLKDIV1]   = 0x5413f855,
> +[NPCM7XX_CLK_PLLCON0]   = 0x00222101 | PLLCON_LOKI,
> +[NPCM7XX_CLK_PLLCON1]   = 0x00202101 | PLLCON_LOKI,
> +[NPCM7XX_CLK_IPSRST1]   = 0x1000,
> +[NPCM7XX_CLK_IPSRST2]   = 0x8000,
> +[NPCM7XX_CLK_CLKEN2]= 0x,
> +[NPCM7XX_CLK_CLKDIV2]   = 0xaa4f8f9f,
> +[NPCM7XX_CLK_CLKEN3]= 0x,
> +[NPCM7XX_CLK_IPSRST3]   = 0x0300,
> +[NPCM7XX_CLK_WD0RCR]= 0x,
> +[NPCM7XX_CLK_WD1RCR]= 0x,
> +[NPCM7XX_CLK_WD2RCR]= 0x,
> +[NPCM7XX_CLK_SWRSTC1]   = 0x0003,
> +[NPCM7XX_CLK_PLLCON2]   = 0x00c02105 | PLLCON_LOKI,
> +[NPCM7XX_CLK_CORSTC]= 0x0403,
> +[NPCM7XX_CLK_PLLCONG]   = 0x01228606 | PLLCON_LOKI,
> +[NPCM7XX_CLK_AHBCKFI]   = 0x00c8,
> +};

Re: [PATCH 4/6] hw/timer: Add NPCM7xx Timer device model

[Joel Stanley]

On Thu, 21 May 2020 at 20:38, Havard Skinnemoen  wrote:
>
> The NPCM730 and NPCM750 SoCs have three timer modules each holding five
> timers and some shared registers (e.g. interrupt status).
>
> Each timer runs at 25 MHz divided by a prescaler, and counts down from a
> configurable initial value to zero. When zero is reached, the interrupt
> flag for the timer is set, and the timer is disabled (one-shot mode) or
> reloaded from its initial value (periodic mode).
>
> This implementation is sufficient to boot a Linux kernel configured for
> NPCM750. Note that the kernel does not seem to actually turn on the
> interrupts.
>
> Reviewed-by: Tyrone Ting 
> Signed-off-by: Havard Skinnemoen 

Reviewed-by: Joel Stanley 

Re: [PATCH 1/6] npcm7xx: Add config symbol

[Joel Stanley]

On Thu, 21 May 2020 at 20:39, Havard Skinnemoen  wrote:
>
> Add a config symbol for the NPCM7xx BMC SoC family that subsequent
> patches can use in Makefiles.
>
> Reviewed-by: Tyrone Ting 
> Signed-off-by: Havard Skinnemoen 

Acked-by: Joel Stanley 


> ---
>  default-configs/arm-softmmu.mak | 1 +
>  hw/arm/Kconfig  | 8 
>  2 files changed, 9 insertions(+)
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 8fc09a4a51..9a94ebd0be 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -27,6 +27,7 @@ CONFIG_GUMSTIX=y
>  CONFIG_SPITZ=y
>  CONFIG_TOSA=y
>  CONFIG_Z2=y
> +CONFIG_NPCM7XX=y
>  CONFIG_COLLIE=y
>  CONFIG_ASPEED_SOC=y
>  CONFIG_NETDUINO2=y
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 5364172537..47d0a3ca43 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -354,6 +354,14 @@ config XLNX_VERSAL
>  select VIRTIO_MMIO
>  select UNIMP
>
> +config NPCM7XX
> +bool
> +select A9MPCORE
> +select ARM_GIC
> +select PL310  # cache controller
> +select SERIAL
> +select UNIMP
> +
>  config FSL_IMX25
>  bool
>  select IMX
> --
> 2.27.0.rc0.183.gde8f92d652-goog
>
>

[Bug 1882350] Re: it always create sdx device when I configure ide device with hdx name

[marshell]

Thanks a lot for the reply.


But from the cmdline of qemu, we can see as following, libvirt passed "-device" 
option with "ide-hd, bus=ide.0" to qemu. I am wondering why qemu received this 
option, but it is still dealing it as scsi bus device instead of ide bus 
device, since with "lssci" cmd, we can see the ide disk we configured in xml. 

>3. from vm.log:

>le=/data2.qcow2,format=qcow2,if=none,id=drive-ide0-0-1 -device ide-
hd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 >-drive
file=/data3_raw.qcow2,format=raw,if=none,id=drive-ide0-1-0 -device ide-
hd,bus=ide.1,unit=0,drive=drive-ide0-1->0,id=ide0-1-0 -netdev t

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

Title:
  it always create sdx device when I configure ide device with hdx name

Status in QEMU:
  New

Bug description:
  I have configured 2 ide disks with name starting with hd, but when the
  vm boots up, it shows disks whose name starting with sd.

  1. ide disks in vm xml:

  



  
  



  

  
  2. in VM:

  sda8:002G  0 disk
  sdb8:16   01G  0 disk

  
  3. from vm.log:

  le=/data2.qcow2,format=qcow2,if=none,id=drive-ide0-0-1 -device ide-
  hd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 -drive
  file=/data3_raw.qcow2,format=raw,if=none,id=drive-ide0-1-0 -device
  ide-hd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -netdev t

  
  4. rpm info: (I got the same issue on 2 diff envs)
  (1) env1
  qemu-kvm-1.5.3-105
  libvirt-3.2.0-14.el7
  (2) env2
  libvirt-5.9.0-1.el8
  qemu-4.1.0-1.el8

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

[PATCH] Makefile: Install qemu-[qmp/ga]-ref.* into the directory "interop"

[Yi Wang]

From: Liao Pingfang 

We need install qemu-[qmp/ga]-ref.* files into the subdirectory of qemu docs: 
interop.

If we visit the following address and click the link to qemu-qmp-ref.html:
https://www.qemu.org/docs/master/interop/bitmaps.html#basic-qmp-usage

It will report following error:
"
Not Found
The requested URL /docs/master/interop/qemu-qmp-ref.html was not found on this 
server.
"

Signed-off-by: Liao Pingfang 
---
 Makefile   | 10 ++
 docs/index.html.in |  4 ++--
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 40e4f76..49dbe7a 100644
--- a/Makefile
+++ b/Makefile
@@ -879,8 +879,9 @@ install-sphinxdocs: sphinxdocs
 install-doc: $(DOCS) install-sphinxdocs
$(INSTALL_DIR) "$(DESTDIR)$(qemu_docdir)"
$(INSTALL_DATA) $(MANUAL_BUILDDIR)/index.html "$(DESTDIR)$(qemu_docdir)"
-   $(INSTALL_DATA) docs/interop/qemu-qmp-ref.html 
"$(DESTDIR)$(qemu_docdir)"
-   $(INSTALL_DATA) docs/interop/qemu-qmp-ref.txt "$(DESTDIR)$(qemu_docdir)"
+   $(INSTALL_DIR) "$(DESTDIR)$(qemu_docdir)/interop"
+   $(INSTALL_DATA) docs/interop/qemu-qmp-ref.html 
"$(DESTDIR)$(qemu_docdir)/interop"
+   $(INSTALL_DATA) docs/interop/qemu-qmp-ref.txt 
"$(DESTDIR)$(qemu_docdir)/interop"
 ifdef CONFIG_POSIX
$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man1"
$(INSTALL_DATA) $(MANUAL_BUILDDIR)/system/qemu.1 
"$(DESTDIR)$(mandir)/man1"
@@ -898,8 +899,9 @@ ifdef CONFIG_TRACE_SYSTEMTAP
 endif
 ifneq (,$(findstring qemu-ga,$(TOOLS)))
$(INSTALL_DATA) $(MANUAL_BUILDDIR)/interop/qemu-ga.8 
"$(DESTDIR)$(mandir)/man8"
-   $(INSTALL_DATA) docs/interop/qemu-ga-ref.html "$(DESTDIR)$(qemu_docdir)"
-   $(INSTALL_DATA) docs/interop/qemu-ga-ref.txt "$(DESTDIR)$(qemu_docdir)"
+   $(INSTALL_DIR) "$(DESTDIR)$(qemu_docdir)/interop"
+   $(INSTALL_DATA) docs/interop/qemu-ga-ref.html 
"$(DESTDIR)$(qemu_docdir)/interop"
+   $(INSTALL_DATA) docs/interop/qemu-ga-ref.txt 
"$(DESTDIR)$(qemu_docdir)/interop"
$(INSTALL_DATA) docs/interop/qemu-ga-ref.7 "$(DESTDIR)$(mandir)/man7"
 endif
 endif
diff --git a/docs/index.html.in b/docs/index.html.in
index e9a1603..6736fa4 100644
--- a/docs/index.html.in
+++ b/docs/index.html.in
@@ -12,8 +12,8 @@
 Tools Guide
 System Emulation Management and 
Interoperability Guide
 System Emulation Guest Hardware 
Specifications
-QMP Reference Manual
-Guest Agent Protocol 
Reference
+QMP Reference 
Manual
+Guest Agent Protocol 
Reference
 
 
 
-- 
2.9.5

Re: [PATCH 0/6] Add Nuvoton NPCM730/NPCM750 SoCs and two BMC machines

[Havard Skinnemoen]

On Thu, May 21, 2020 at 12:21 PM Havard Skinnemoen 
wrote:

> This patch series models enough of the Nuvoton NPCM730 and NPCM750 SoCs to
> boot
> a minimal Linux kernel. This includes device models for:
>

Does anyone have comments on this series? I'm currently finishing up a
second patch series that adds flash support and a few other things so qemu
can boot a full OpenBMC flash image built for npcm7xx.

If you prefer, I can combine them both into one series and send it to the
list.

This series can be found here:
https://patchwork.kernel.org/project/qemu-devel/list/?series=291809

Thanks,

Havard

Re: [PATCH v5 13/13] qcow2_format.py: dump bitmaps header extension

[Eric Blake]

On 6/6/20 3:18 AM, Vladimir Sementsov-Ogievskiy wrote:

Add class for bitmap extension and dump its fields. Further work is to
dump bitmap directory.

Test new functionality inside 291 iotest.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Andrey Shinkevich 
---



@@ -88,6 +90,8 @@ $QEMU_IMG bitmap --merge tmp -f $IMGFMT "$TEST_IMG" b0
  $QEMU_IMG bitmap --remove --image-opts \
  driver=$IMGFMT,file.driver=file,file.filename="$TEST_IMG" tmp
  $QEMU_IMG info "$TEST_IMG" | _filter_img_info --format-specific
+echo "Check resulting qcow2 header extensions:"
+$PYTHON qcow2.py "$TEST_IMG" dump-header-exts


Context conflict with my pending patch to resolve an issue reported by 
Kevin:


https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg02057.html

but that is easy enough to resolve.

I'll be queuing this series through my bitmaps tree, hopefully with a 
pull request on Tuesday.


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

Re: [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20200608164357.25065-1-filip.boz...@syrmia.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20200608164357.25065-1-filip.boz...@syrmia.com
Subject: [PATCH v2 0/6] Add strace support for printing arguments of selected 
syscalls
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
dc47702 linux-user: Add strace support for printing arguments of fallocate()
8478ea8 linux-user: Add strace support for printing arguments of 
chown()/lchown()
f287b7f linux-user: Add strace support for printing arguments of lseek()
615a5d8 linux-user: Add strace support for printing argument of syscalls used 
for extended attributes
b0acbe9 linux-user: Add strace support for a group of syscalls
3370ce2 linux-user: Extend strace support to enable argument printing after 
syscall execution

=== OUTPUT BEGIN ===
1/6 Checking commit 3370ce28f836 (linux-user: Extend strace support to enable 
argument printing after syscall execution)
2/6 Checking commit b0acbe9ef27d (linux-user: Add strace support for a group of 
syscalls)
3/6 Checking commit 615a5d8e9d21 (linux-user: Add strace support for printing 
argument of syscalls used for extended attributes)
4/6 Checking commit f287b7f80630 (linux-user: Add strace support for printing 
arguments of lseek())
5/6 Checking commit 8478ea817695 (linux-user: Add strace support for printing 
arguments of chown()/lchown())
6/6 Checking commit dc477027df64 (linux-user: Add strace support for printing 
arguments of fallocate())
ERROR: storage class should be at the beginning of the declaration
#69: FILE: linux-user/strace.c:1147:
+UNUSED static struct flags falloc_flags[] = {

total: 1 errors, 0 warnings, 104 lines checked

Patch 6/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200608164357.25065-1-filip.boz...@syrmia.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH v5 08/13] qcow2_format.py: separate generic functionality of structure classes

[Eric Blake]

On 6/6/20 3:18 AM, Vladimir Sementsov-Ogievskiy wrote:

We are going to introduce more Qcow2 structure types, defined like
QcowHeader. Move generic functionality into base class to be reused for
further structure classes.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Andrey Shinkevich 
---
  tests/qemu-iotests/qcow2_format.py | 101 +++--
  1 file changed, 66 insertions(+), 35 deletions(-)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index 28f2bfa63b..898d388b8a 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -1,5 +1,7 @@
  # Library for manipulations with qcow2 image
  #
+# Copyright (c) 2020 Virtuozzo International GmbH.


Deferring the addition of this line until this point in the series is 
fine. Depending on the answer I get to adding any other copyright line 
in 2/ and 3/13, or if that is deferred to later, we can figure out the 
merge conflict by keeping both.


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

Re: [PATCH v2 06/13] tcg: call qemu_spin_destroy for tb->jmp_lock

[Robert Foley]

On Mon, 8 Jun 2020 at 10:44, Alex Bennée  wrote:

> > +static void tcg_region_tree_reset_all(tb_destroy_func tb_destroy)
> >  {
> >  size_t i;
> >
> > @@ -510,6 +519,10 @@ static void tcg_region_tree_reset_all(void)
> >  for (i = 0; i < region.n; i++) {
> >  struct tcg_region_tree *rt = region_trees + i * tree_size;
> >
> > +if (tb_destroy != NULL) {
> > +g_tree_foreach(rt->tree, tcg_region_tree_traverse, tb_destroy);
> > +}
> > +
>
> Isn't tb_destroy always set? We could assert that is the case rather
> than make the cleaning up conditional.

I agree, tb_destroy seems to always be set, so the assert would be reasonable.

>
> >  /* Increment the refcount first so that destroy acts as a reset */
> >  g_tree_ref(rt->tree);
> >  g_tree_destroy(rt->tree);
> > @@ -586,7 +599,7 @@ static inline bool 
> > tcg_region_initial_alloc__locked(TCGContext *s)
> >  }
> >
> >  /* Call from a safe-work context */
> > -void tcg_region_reset_all(void)
> > +void tcg_region_reset_all(tb_destroy_func tb_destroy)
> >  {
> >  unsigned int n_ctxs = atomic_read(&n_tcg_ctxs);
> >  unsigned int i;
> > @@ -603,7 +616,7 @@ void tcg_region_reset_all(void)
> >  }
> >  qemu_mutex_unlock(®ion.lock);
> >
> > -tcg_region_tree_reset_all();
> > +tcg_region_tree_reset_all(tb_destroy);
>
> Could you name the variables of type tb_destroy_func differently as
> although the variable is only ever tb_destroy the function it gets
> confusing real quick when trying to grep for stuff. Maybe tbd_fn?
>
> That said given the single usage why a function pointer? Would we be
> just as well served by an exposed public function call from the
> appropriate places?

Good point.  Unless we see an imminent need to pass different values,
then it seems
reasonable to just use the public function call and remove the need for
the function pointer.

Thanks & Regards,
-Rob


>
> Richard do you have a view here?
>
> --
> Alex Bennée

Re: [PATCH v5 03/13] qcow2.py: move qcow2 format classes to separate module

[Eric Blake]

On 6/6/20 3:17 AM, Vladimir Sementsov-Ogievskiy wrote:

We are going to enhance qcow2 format parsing by adding more structure
classes. Let's split format parsing from utility code.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  tests/qemu-iotests/qcow2.py| 160 +-
  tests/qemu-iotests/qcow2_format.py | 173 +
  2 files changed, 177 insertions(+), 156 deletions(-)
  create mode 100644 tests/qemu-iotests/qcow2_format.py




+++ b/tests/qemu-iotests/qcow2_format.py
@@ -0,0 +1,173 @@
+# Library for manipulations with qcow2 image
+#


Any copyright line added in 2/13 should also be added here.  Again, that 
can be a followup for authorship reasons if we decide what copyright 
line is best.


Otherwise, this is a clean code motion patch where the difference in 
line length is the boilerplate header and the change to imports.


Reviewed-by: Eric Blake 

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

Re: [PATCH 6/7] target/i386: reimplement fprem1 using floatx80 operations

[Alex Bennée]

Joseph Myers  writes:

> On Mon, 8 Jun 2020, Alex Bennée wrote:
>
>> > +uint8_t old_flags = save_exception_flags(env);
>> 
>> Hmm where did this come from:
>
> This series assumes all my other recent x87 fixes (11 such patches in 
> three series that aren't yet on master, there's also a single patch for 
> pcmpxstrx which is independent of those) are already present.

Ahh - it's ok to keep a patch that is currently queued in your series
until such time as it's merged. You can also just reference the original
series in your cover letter or supply a branch reference to aid with
application.

-- 
Alex Bennée

Re: [PATCH v5 02/13] qcow2.py: add licensing blurb

[Eric Blake]

On 6/6/20 3:17 AM, Vladimir Sementsov-Ogievskiy wrote:

Add classic heading, which is missing here. Keep copyright place empty,
for anyone who have added (or will add) some intellectual property
here.


It's not so much intellectual property (since that term is at odds with 
open source), but authorship rights.


Looking at git history, the file has been touched by:

Kevin Wolf
Stefan Hajnoczi (while at IBM)
Eduardo Habkost
Max Reitz
Philippe Mathieu-Daudé
Paolo Bonzini

where Stefan was the only contributor without a redhat.com address at 
the time.  So if anything, a Red Hat copyright is most likely; but you 
are also correct that it is incorrect to add a copyright line on someone 
else's behalf without their permission.




Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  tests/qemu-iotests/qcow2.py | 16 
  1 file changed, 16 insertions(+)

diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index d99f4ee3e8..2da434a013 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -1,4 +1,20 @@
  #!/usr/bin/env python3
+#
+# Manipulations with qcow2 image
+#


I've cc'd all prior authors; if Kevin agrees, and unless anyone speaks 
up to the contrary, I'm willing to add:


# Copyright (C) 2012 Red Hat, Inc.

for Kevin's initial contribution, without worrying about subsequent 
contributions.



+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
  
  import sys

  import struct



Adding a copyright line could be a followup patch, so in the meantime, 
making what was previously an implicit license now explicit is fine even 
if it is odd to assert GPL without also asserting Copyright.


Reviewed-by: Eric Blake 

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

Re: [PATCH v5 01/13] qcow2.py: python style fixes

[Eric Blake]

On 6/6/20 3:17 AM, Vladimir Sementsov-Ogievskiy wrote:

Fix flake8 complaints. Leave the only chunk of lines over 79 characters:


Stale sentence since you fixed that; on my Fedora 32 machine, with 
flake8 3.7.7, this patch silences all warnings.



initialization of cmds variable. Leave it for another day, when it
should be refactored to utilize argparse instead of hand-written
parsing.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  tests/qemu-iotests/qcow2.py | 95 ++---
  1 file changed, 56 insertions(+), 39 deletions(-)


Reviewed-by: Eric Blake 
Tested-by: Eric Blake 

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

[PATCH 0/2] Fix couple of issues with AMD topology

[Babu Moger]

This series fixes couple of issues with recent topology related code.
1. Fix uninitialized memory with -device and CPU hotplug
2. Simplify CPUID_8000_001E to generalize the support for higher
   number of cores and nodes

Here are the threads discussing the issue.
https://lore.kernel.org/qemu-devel/20200602175212.gh577...@habkost.net/
https://lore.kernel.org/qemu-devel/20200602175212.gh577...@habkost.net/
---

Babu Moger (2):
  hw/386: Fix uninitialized memory with -device and CPU hotplug
  i386: Simplify CPUID_8000_001E for AMD


 hw/i386/pc.c   |2 +
 include/hw/i386/topology.h |   11 ++
 target/i386/cpu.c  |   78 +---
 3 files changed, 50 insertions(+), 41 deletions(-)

--

[PATCH 1/2] hw/386: Fix uninitialized memory with -device and CPU hotplug

[Babu Moger]

Noticed the following command failure while testing CPU hotplug.

$ qemu-system-x86_64 -machine q35,accel=kvm -smp 1,maxcpus=2,
  cores=1, threads=1,sockets=2 -cpu EPYC -device EPYC-x86_64-
  cpu,core-id=0,socket-id=1,thread-id=0

  qemu-system-x86_64: -device EPYC-x86_64-cpu,core-id=0,socket-id=1,
  thread-id=0: Invalid CPU [socket: 21855, die: 0, core: 0, thread: 0]
  with APIC ID 21855, valid index range 0:1

This happens because APIC ID is calculated using uninitialized memory.
This is happening after the addition of new field node_id in X86CPUTopoIDs
structure. The node_id field is uninitialized while calling
apicid_from_topo_ids. The problem is discussed in the thread below.
https://lore.kernel.org/qemu-devel/20200602171838.gg577...@habkost.net/

Fix the problem by initializing the node_id properly.

Signed-off-by: Babu Moger 
---
 hw/i386/pc.c   |2 ++
 include/hw/i386/topology.h |   11 +++
 2 files changed, 13 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2128f3d6fe..974cc30891 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1585,6 +1585,8 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
 topo_ids.die_id = cpu->die_id;
 topo_ids.core_id = cpu->core_id;
 topo_ids.smt_id = cpu->thread_id;
+topo_ids.node_id = cpu_x86_use_epyc_apic_id_encoding(ms->cpu_type) ?
+   x86_node_id_for_epyc(&topo_info, &topo_ids) : 0;
 cpu->apic_id = x86ms->apicid_from_topo_ids(&topo_info, &topo_ids);
 }
 
diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
index 07239f95f4..ee4deb84c4 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -140,6 +140,17 @@ static inline unsigned 
apicid_pkg_offset_epyc(X86CPUTopoInfo *topo_info)
apicid_node_width_epyc(topo_info);
 }
 
+static inline unsigned x86_node_id_for_epyc(X86CPUTopoInfo *topo_info,
+const X86CPUTopoIDs *topo_ids)
+{
+unsigned nr_nodes = MAX(topo_info->nodes_per_pkg, 1);
+unsigned cores_per_node = DIV_ROUND_UP((topo_info->dies_per_pkg *
+topo_info->cores_per_die *
+topo_info->threads_per_core),
+nr_nodes);
+
+return (topo_ids->core_id / cores_per_node) % nr_nodes;
+}
 /*
  * Make APIC ID for the CPU based on Pkg_ID, Core_ID, SMT_ID
  *

[PATCH 2/2] i386: Simplify CPUID_8000_001E for AMD

[Babu Moger]

apic_id contains all the information required to build
CPUID_8000_001E. core_id and node_id is already part of
apic_id generated by x86_topo_ids_from_apicid_epyc.
Also remove the restriction on number bits on core_id and
node_id.

Remove all the hardcoded values and replace with generalized
fields.

Refer the Processor Programming Reference (PPR) documentation
available from the bugzilla Link below.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
Signed-off-by: Babu Moger 
---
 target/i386/cpu.c |   78 +
 1 file changed, 37 insertions(+), 41 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3733d9a279..166d2f2bde 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -388,57 +388,53 @@ static void encode_topo_cpuid801e(X86CPUTopoInfo 
*topo_info, X86CPU *cpu,
 {
 X86CPUTopoIDs topo_ids = {0};
 unsigned long nodes = MAX(topo_info->nodes_per_pkg, 1);
-int shift;
 
 x86_topo_ids_from_apicid_epyc(cpu->apic_id, topo_info, &topo_ids);
 
 *eax = cpu->apic_id;
+
 /*
+ * CPUID_Fn801E_EBX [Core Identifiers] (CoreId)
+ * Read-only. Reset: _h.
+ * See Core::X86::Cpuid::ExtApicId.
+ * Core::X86::Cpuid::CoreId_lthree[1:0]_core[3:0]_thread[1:0];
  * CPUID_Fn801E_EBX
- * 31:16 Reserved
- * 15:8  Threads per core (The number of threads per core is
- *   Threads per core + 1)
- *  7:0  Core id (see bit decoding below)
- *   SMT:
- *   4:3 node id
- * 2 Core complex id
- *   1:0 Core id
- *   Non SMT:
- *   5:4 node id
- * 3 Core complex id
- *   1:0 Core id
+ * Bits Description
+ * 31:16 Reserved.
+ * 15:8 ThreadsPerCore: threads per core. Read-only. Reset: XXh.
+ *  The number of threads per core is ThreadsPerCore+1.
+ *  7:0 CoreId: core ID. Read-only. Reset: XXh.
+ *
+ *  NOTE: CoreId is already part of apic_id. Just use it. We can
+ *  use all the 8 bits to represent the core_id here.
  */
-*ebx = ((topo_info->threads_per_core - 1) << 8) | (topo_ids.node_id << 3) |
-(topo_ids.core_id);
+*ebx = ((topo_info->threads_per_core - 1) << 8) | (topo_ids.core_id & 
0xFF);
+
 /*
+ * CPUID_Fn801E_ECX [Node Identifiers] (NodeId)
+ * Read-only. Reset: _0XXXh.
+ * Core::X86::Cpuid::NodeId_lthree[1:0]_core[3:0]_thread[1:0];
  * CPUID_Fn801E_ECX
- * 31:11 Reserved
- * 10:8  Nodes per processor (Nodes per processor is number of nodes + 1)
- *  7:0  Node id (see bit decoding below)
- * 2  Socket id
- *   1:0  Node id
+ * Bits Description
+ * 31:11 Reserved.
+ * 10:8 NodesPerProcessor: Node per processor. Read-only. Reset: XXXb.
+ *  ValidValues:
+ *  Value Description
+ *  000b  1 node per processor.
+ *  001b  2 nodes per processor.
+ *  010b Reserved.
+ *  011b 4 nodes per processor.
+ *  111b-100b Reserved.
+ *  7:0 NodeId: Node ID. Read-only. Reset: XXh.
+ *
+ * NOTE: Hardware reserves 3 bits for number of nodes per processor.
+ * But users can create more nodes than the actual hardware can
+ * support. To genaralize we can use all the upper 8 bits for nodes.
+ * NodeId is combination of node and socket_id which is already decoded
+ * in apic_id. Just use it by shifting.
  */
-if (nodes <= 4) {
-*ecx = ((nodes - 1) << 8) | (topo_ids.pkg_id << 2) | topo_ids.node_id;
-} else {
-/*
- * Node id fix up. Actual hardware supports up to 4 nodes. But with
- * more than 32 cores, we may end up with more than 4 nodes.
- * Node id is a combination of socket id and node id. Only requirement
- * here is that this number should be unique accross the system.
- * Shift the socket id to accommodate more nodes. We dont expect both
- * socket id and node id to be big number at the same time. This is not
- * an ideal config but we need to to support it. Max nodes we can have
- * is 32 (255/8) with 8 cores per node and 255 max cores. We only need
- * 5 bits for nodes. Find the left most set bit to represent the total
- * number of nodes. find_last_bit returns last set bit(0 based). Left
- * shift(+1) the socket id to represent all the nodes.
- */
-nodes -= 1;
-shift = find_last_bit(&nodes, 8);
-*ecx = (nodes << 8) | (topo_ids.pkg_id << (shift + 1)) |
-   topo_ids.node_id;
-}
+*ecx = ((nodes - 1) << 8) |
+((cpu->apic_id >> apicid_node_offset_epyc(topo_info)) & 0xFF);
 *edx = 0;
 }
 

Re: [RFC v3 7/8] vhost-vdpa: introduce vhost-vdpa backend

[Eric Blake]

On 5/29/20 9:06 AM, Cindy Lu wrote:

From: Tiwei Bie 


The original author is Tiwei Bie...



Currently we have 2 types of vhost backends in QEMU: vhost kernel and
vhost-user. The above patch provides a generic device for vDPA purpose,
this vDPA device exposes to user space a non-vendor-specific configuration
interface for setting up a vhost HW accelerator, this patch set introduces
a third vhost backend called vhost-vdpa based on the vDPA interface.

Vhost-vdpa usage:

   qemu-system-x86_64 -cpu host -enable-kvm \
 ..
   -netdev type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-id,id=vhost-vdpa0 \
   -device virtio-net-pci,netdev=vhost-vdpa0,page-per-vq=on \

Co-Authored-By: Lingshan zhu 
Signed-off-by: Cindy Lu 
---


...but there is no S-o-b here.  Also, Co-Authored-By is an unusual tag; 
it's just as easy to spell it Signed-off-by even for co-authors.


[Pardon my delicacy in wording my response; I unfortunately lack enough 
cultural context to know a preferred name or even gender-correct 
pronouns for referring to the authors in shorthand]


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

Re: [PATCH v2 01/13] configure: add --enable-tsan flag + fiber annotations for coroutine-ucontext

[Robert Foley]

On Mon, 8 Jun 2020 at 09:39, Alex Bennée  wrote:

> > -static void finish_switch_fiber(void *fake_stack_save)
> > +/* QEMU_ALWAYS_INLINE only does so if __OPTIMIZE__, so we cannot use it. */
> > +static inline __attribute__((always_inline))
> > +void on_new_fiber(CoroutineUContext *co)
> > +{
>
> We could put a tracepoint here at something like trace_new_fibre() but I
> suspect for following what's going on you could probably just have
> tracepoints in the higher coroutine functions and leave the fibre stuff
> as purely a CONFIG_TSAN detail.
>
> Please we wouldn't have to ague about American vs British spelling for
> the tracepoints ;-)
>
> 
>
> Otherwise without the UC_TRACE verbiage:
>
> Reviewed-by: Alex Bennée 

Thanks for the review.  Sounds good, we will remove the fibre related
traces. :-)

Thanks & Regards,
-Rob
>
> --
> Alex Bennée

[PATCH] iotests: Fix 291 across more file systems

[Eric Blake]

Depending on the granularity of holes and amount of metadata consumed
by a file, the 'disk size:' number of 'qemu-img info' is not reliable.
Adjust our test to use a different set of filters to avoid spurious
failures.

Reported-by: Kevin Wolf 
Fixes: cf2d1203dc
Signed-off-by: Eric Blake 
---
 tests/qemu-iotests/291 | 4 ++--
 tests/qemu-iotests/291.out | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/291 b/tests/qemu-iotests/291
index 3ca83b9cd1f7..910326b0ec55 100755
--- a/tests/qemu-iotests/291
+++ b/tests/qemu-iotests/291
@@ -77,7 +77,7 @@ echo

 # Only bitmaps from the active layer are copied
 $QEMU_IMG convert --bitmaps -O qcow2 "$TEST_IMG.orig" "$TEST_IMG"
-$QEMU_IMG info "$TEST_IMG" | _filter_img_info --format-specific
+_img_info --format-specific
 # But we can also merge in bitmaps from other layers.  This test is a bit
 # contrived to cover more code paths, in reality, you could merge directly
 # into b0 without going through tmp
@@ -87,7 +87,7 @@ $QEMU_IMG bitmap --add --merge b0 -b "$TEST_IMG.base" -F 
$IMGFMT \
 $QEMU_IMG bitmap --merge tmp -f $IMGFMT "$TEST_IMG" b0
 $QEMU_IMG bitmap --remove --image-opts \
 driver=$IMGFMT,file.driver=file,file.filename="$TEST_IMG" tmp
-$QEMU_IMG info "$TEST_IMG" | _filter_img_info --format-specific
+_img_info --format-specific

 echo
 echo "=== Check bitmap contents ==="
diff --git a/tests/qemu-iotests/291.out b/tests/qemu-iotests/291.out
index 8c62017567e9..9f661515b417 100644
--- a/tests/qemu-iotests/291.out
+++ b/tests/qemu-iotests/291.out
@@ -24,7 +24,7 @@ qemu-img: Format driver 'raw' does not support bitmaps
 image: TEST_DIR/t.IMGFMT
 file format: IMGFMT
 virtual size: 10 MiB (10485760 bytes)
-disk size: 4.39 MiB
+cluster_size: 65536
 Format specific information:
 compat: 1.1
 compression type: zlib
@@ -44,7 +44,7 @@ Format specific information:
 image: TEST_DIR/t.IMGFMT
 file format: IMGFMT
 virtual size: 10 MiB (10485760 bytes)
-disk size: 4.48 MiB
+cluster_size: 65536
 Format specific information:
 compat: 1.1
 compression type: zlib
-- 
2.27.0

Re: [RFC PATCH 22/35] hw/m68k/mcf520x: Emit warning when old code is used

[Thomas Huth]

Am Mon,  8 Jun 2020 18:00:31 +0200
schrieb Philippe Mathieu-Daudé :

> This code hasn't been QOM'ified yet. Warn the user.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/m68k/mcf5206.c | 5 +
>  hw/m68k/mcf5208.c | 3 +++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c
> index a2fef04f8e..ec0d176674 100644
> --- a/hw/m68k/mcf5206.c
> +++ b/hw/m68k/mcf5206.c
> @@ -16,6 +16,7 @@
>  #include "qemu/timer.h"
>  #include "hw/ptimer.h"
>  #include "sysemu/sysemu.h"
> +#include "hw/qdev-deprecated.h"
>  
>  /* General purpose timer module.  */
>  typedef struct {
> @@ -144,6 +145,8 @@ static m5206_timer_state
> *m5206_timer_init(qemu_irq irq) {
>  m5206_timer_state *s;
>  
> +qdev_warn_deprecated_function_used();
> +
>  s = g_new0(m5206_timer_state, 1);
>  s->timer = ptimer_init(m5206_timer_trigger, s,
> PTIMER_POLICY_DEFAULT); s->irq = irq;
> @@ -566,6 +569,8 @@ qemu_irq *mcf5206_init(MemoryRegion *sysmem,
> uint32_t base, M68kCPU *cpu) m5206_mbar_state *s;
>  qemu_irq *pic;
>  
> +qdev_warn_deprecated_function_used();
> +
>  s = g_new0(m5206_mbar_state, 1);

Ok, it's quite obvious what you refer to here...

>  memory_region_init_io(&s->iomem, NULL, &m5206_mbar_ops, s,
> diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
> index 2ab9701ad6..72627f6833 100644
> --- a/hw/m68k/mcf5208.c
> +++ b/hw/m68k/mcf5208.c
> @@ -26,6 +26,7 @@
>  #include "hw/sysbus.h"
>  #include "elf.h"
>  #include "exec/address-spaces.h"
> +#include "hw/qdev-deprecated.h"
>  
>  #define SYS_FREQ 1
>  
> @@ -191,6 +192,8 @@ static void mcf5208_sys_init(MemoryRegion
> *address_space, qemu_irq *pic) m5208_timer_state *s;
>  int i;
>  
> +qdev_warn_deprecated_function_used();
> +
>  /* SDRAMC.  */
>  memory_region_init_io(iomem, NULL, &m5208_sys_ops, NULL,
> "m5208-sys", 0x4000); memory_region_add_subregion(address_space,
> 0xfc0a8000, iomem);

... but it is not so obvious what you refer to here. I think that new
function should maybe have a "char *what" parameter that contains the
name of the struct you refer to. Or at least add a comment in front of
the function with a short description?

 Thomas

Re: [PATCH v1 2/2] nvme: allow cmb and pmr to be enabled on same device

[Andrzej Jakowski]

On 6/8/20 1:08 AM, Klaus Jensen wrote:
> On Jun  5 11:10, Andrzej Jakowski wrote:
>> So far it was not possible to have CMB and PMR emulated on the same
>> device, because BAR2 was used exclusively either of PMR or CMB. This
>> patch places CMB at BAR4 offset so it not conflicts with MSI-X vectors.
>>
> 
> Hi Andrzej,
> 
> Thanks for doing this, it's a nice addition!
> 
> Though, I would prefer that the table and pba was located in BAR0 and
> keeping BAR4 for exclusive CMB use. I'm no expert on this, but is it ok
> to have the table and pba in prefetchable memory? Having it "together"
> with the other controller-level configuration memory just feels more
> natural to me, but I'm not gonna put my foot down.
Hi Klaus,

Thx for your feedback!
I don't think it matters if MSIX table is in prefetchable vs 
non-prefetchable memory. 
My understanding is that spec allows MSIX and PBA to be in any BAR and
offset. I understand your preference and at the same time think that
since it is not in violation of the spec why don't we leave it as-is?
Does anybody know what's typical approach for real devices?
> 
> Using BAR0 would also slightly simplify the patch since no changes would
> be required for the CMB path.
> 
> Also, can you rebase this on Kevin's block branch? There are a bunch of
> refactoring patches that changes the realization code, so this patch
> doesn't apply at all.
Yep will reabse it.
> 
>> Signed-off-by: Andrzej Jakowski 
>> ---
>>  hw/block/nvme.c  | 127 +--
>>  hw/block/nvme.h  |   3 +-
>>  include/block/nvme.h |   4 +-
>>  3 files changed, 91 insertions(+), 43 deletions(-)
>>
>> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
>> index f0b45704be..353cf20e0a 100644
>> --- a/hw/block/nvme.c
>> +++ b/hw/block/nvme.c
>> @@ -22,12 +22,12 @@
>>   *  [pmrdev=,] \
>>   *  num_queues=
>>   *
>> - * Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at
>> - * offset 0 in BAR2 and supports only WDS, RDS and SQS for now.
>> + * Note cmb_size_mb denotes size of CMB in MB. CMB when configured is 
>> assumed
>> + * to be resident in BAR4 at certain offset - this is because BAR4 is also
>> + * used for storing MSI-X table that is available at offset 0 in BAR4.
>>   *
>> - * cmb_size_mb= and pmrdev= options are mutually exclusive due to limitation
>> - * in available BAR's. cmb_size_mb= will take precedence over pmrdev= when
>> - * both provided.
>> + * pmrdev is assumed to be resident in BAR2. When configured it consumes 
>> whole
>> + * BAR2 exclusively.
> 
> Actually it uses both BAR2 and BAR3 since its 64 bits.
Correct. That's what I implied here w/o actual verbiage. I can extend it
to add that information.
> 
>> @@ -1342,6 +1346,71 @@ static const MemoryRegionOps nvme_cmb_ops = {
>>  },
>>  };
>>  
>> +#define NVME_MSIX_BIR (4)
>> +static void nvme_bar4_init(PCIDevice *pci_dev)
>> +{
>> +NvmeCtrl *n = NVME(pci_dev);
>> +int status;
>> +uint64_t bar_size = 4096;
>> +uint32_t nvme_pba_offset = bar_size / 2;
>> +uint32_t nvme_pba_size = QEMU_ALIGN_UP(n->num_queues, 64) / 8;
>> +uint32_t cmb_size_units;
>> +
>> +if (n->num_queues * PCI_MSIX_ENTRY_SIZE > nvme_pba_offset) {
>> +nvme_pba_offset = n->num_queues * PCI_MSIX_ENTRY_SIZE;
>> +}
>> +
>> +if (nvme_pba_offset + nvme_pba_size > 4096) {
>> +bar_size = nvme_pba_offset + nvme_pba_size;
>> +}
>> +
> 
> This is migration compatibility stuff that is not needed because the
> nvme device is unmigratable anyway.
I don't understand that comment. Could you please explain more?
 

Re: [PATCH v2] hw/openrisc/openrisc_sim: Add assertion to silence GCC warning

[Richard Henderson]

On 6/8/20 9:06 AM, Philippe Mathieu-Daudé wrote:
> When compiling with GCC 10 (Fedora 32) using CFLAGS=-O2 we get:
> 
> CC  or1k-softmmu/hw/openrisc/openrisc_sim.o
>   hw/openrisc/openrisc_sim.c: In function ‘openrisc_sim_init’:
>   hw/openrisc/openrisc_sim.c:87:42: error: ‘cpu_irqs[0]’ may be used 
> uninitialized in this function [-Werror=maybe-uninitialized]
>  87 | sysbus_connect_irq(s, i, cpu_irqs[i][irq_pin]);
> |  ^~~
> 
> While humans can tell smp_cpus will always be in the [1, 2] range,
> (openrisc_sim_machine_init sets mc->max_cpus = 2), the compiler
> can't.
> 
> Add an assertion to give the compiler a hint there's no use of
> uninitialized data.
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1874073
> Reported-by: Martin Liška 
> Suggested-by: Peter Maydell 
> Reviewed-by: Thomas Huth 
> Tested-by: Eric Blake 
> Reviewed-by: Eric Blake 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> v2: Fixed typo in subject (eblake)
> Supersedes: <20200608071409.17024-1-phi...@redhat.com>
> ---
>  hw/openrisc/openrisc_sim.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Richard Henderson 

r~

Re: fpu/softfloat: a question on BFloat 16 support on QEMU

[Richard Henderson]

On 6/8/20 5:53 AM, LIU Zhiwei wrote:
> Hi Richard,
> 
> I am doing bfloat16 support on QEMU.
> 
> Once I tried to reuse float32 interface, but I couldn't properly process
> rounding in some insns like fadd.
> 
> What's your opinion about it? Should I expand the fpu/softfloat?

Yes, we need to expand fpu/softfloat.

You'll want something like

static const FloatFmt bfloat16_params = {
FLOAT_PARAMS(8, 7)
};

(This would be the Arm and x86 definition, anyway; hopefully risc-v is the 
same.)

And then add all of the other interface functions that you need to use that
parameter.

FWIW, it appears that Arm only requires:

  float32_to_bfloat16
  bfloat16_mul
  bfloat16_add

and I could even get away with only float32_to_bfloat16, since technically
Arm's BFAdd and BFMul psuedo-code are implemented in terms of single-precision
arithmetic, followed by a round-to-odd into BFloat16.


r~

Re: [PATCH v6 5/5] iotests: Add test 291 to for qemu-img bitmap coverage

[Eric Blake]

On 6/8/20 2:17 PM, Kevin Wolf wrote:

Am 21.05.2020 um 21:21 hat Eric Blake geschrieben:

Add a new test covering the 'qemu-img bitmap' subcommand, as well as
'qemu-img convert --bitmaps', both added in recent patches.

Signed-off-by: Eric Blake 
Reviewed-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 


This one fails for me. We need to filter out "disk size" because it
depends on the filesystem.


Will fix. Thanks for the heads up.

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

Re: [PATCH 1/3] hw/sparc/leon3: Map the UART device unconditionally

[Artyom Tarasenko]

On Mon, Jun 8, 2020 at 7:28 PM Fred Konrad  wrote:
>
>
>
> Le 6/8/20 à 7:21 PM, Philippe Mathieu-Daudé a écrit :
> > The UART is present on the chipset regardless there is a
> > character device connected to it. Map it unconditionally.
> >
> > Signed-off-by: Philippe Mathieu-Daudé 
> > ---
> >   hw/sparc/leon3.c | 18 --
> >   1 file changed, 8 insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
> > index 8f024dab7b..cc55117dec 100644
> > --- a/hw/sparc/leon3.c
> > +++ b/hw/sparc/leon3.c
> > @@ -339,16 +339,14 @@ static void leon3_generic_hw_init(MachineState 
> > *machine)
> >   0, LEON3_TIMER_IRQ, GRLIB_APBIO_AREA);
> >
> >   /* Allocate uart */
> > -if (serial_hd(0)) {
> > -dev = qdev_create(NULL, TYPE_GRLIB_APB_UART);
> > -qdev_prop_set_chr(dev, "chrdev", serial_hd(0));
> > -qdev_init_nofail(dev);
> > -sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_UART_OFFSET);
> > -sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, 
> > cpu_irqs[LEON3_UART_IRQ]);
> > -grlib_apb_pnp_add_entry(apb_pnp, LEON3_UART_OFFSET, 0xFFF,
> > -GRLIB_VENDOR_GAISLER, GRLIB_APBUART_DEV, 1,
> > -LEON3_UART_IRQ, GRLIB_APBIO_AREA);
> > -}
> > +dev = qdev_create(NULL, TYPE_GRLIB_APB_UART);
> > +qdev_prop_set_chr(dev, "chrdev", serial_hd(0));
> > +qdev_init_nofail(dev);
> > +sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_UART_OFFSET);
> > +sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, cpu_irqs[LEON3_UART_IRQ]);
> > +grlib_apb_pnp_add_entry(apb_pnp, LEON3_UART_OFFSET, 0xFFF,
> > +GRLIB_VENDOR_GAISLER, GRLIB_APBUART_DEV, 1,
> > +LEON3_UART_IRQ, GRLIB_APBIO_AREA);
> >   }
> >
> >   static void leon3_generic_machine_init(MachineClass *mc)
> >
>
> Reviewed-by: KONRAD Frederic 

Acked-by: Artyom Tarasenko 




--
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [PATCH 2/3] hw/sparc64/niagara: Remove duplicated NIAGARA_UART_BASE definition

[Artyom Tarasenko]

On Mon, Jun 8, 2020 at 7:21 PM Philippe Mathieu-Daudé  wrote:
>
> NIAGARA_UART_BASE is already defined few lines earlier.
>
> Signed-off-by: Philippe Mathieu-Daudé 

Thanks!
Reviewed-by: Artyom Tarasenko 

> ---
>  hw/sparc64/niagara.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/hw/sparc64/niagara.c b/hw/sparc64/niagara.c
> index ab5ef8c5b3..201fb05d50 100644
> --- a/hw/sparc64/niagara.c
> +++ b/hw/sparc64/niagara.c
> @@ -68,7 +68,6 @@ typedef struct NiagaraBoardState {
>
>  #define NIAGARA_VDISK_BASE  0x1f4000ULL
>  #define NIAGARA_RTC_BASE0xfff0c1fff8ULL
> -#define NIAGARA_UART_BASE   0x1f1000ULL
>
>  /* Firmware layout
>   *
> --
> 2.21.3
>


-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [PATCH 3/3] hw/sparc64/niagara: Map the UART device unconditionally

[Artyom Tarasenko]

On Mon, Jun 8, 2020 at 7:21 PM Philippe Mathieu-Daudé  wrote:
>
> The UART is present on the machine regardless there is a
> character device connected to it. Map it unconditionally.
>
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Artyom Tarasenko 

> ---
>  hw/sparc64/niagara.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/sparc64/niagara.c b/hw/sparc64/niagara.c
> index 201fb05d50..a87d55f6bb 100644
> --- a/hw/sparc64/niagara.c
> +++ b/hw/sparc64/niagara.c
> @@ -151,10 +151,8 @@ static void niagara_init(MachineState *machine)
>  exit(1);
>  }
>  }
> -if (serial_hd(0)) {
> -serial_mm_init(sysmem, NIAGARA_UART_BASE, 0, NULL, 115200,
> -   serial_hd(0), DEVICE_BIG_ENDIAN);
> -}
> +serial_mm_init(sysmem, NIAGARA_UART_BASE, 0, NULL,
> +   115200, serial_hd(0), DEVICE_BIG_ENDIAN);
>  create_unimplemented_device("sun4v-iob", NIAGARA_IOBBASE, 
> NIAGARA_IOBSIZE);
>  sun4v_rtc_init(NIAGARA_RTC_BASE);
>  }
> --
> 2.21.3
>


-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [PATCH] qcow2: Tweak comments on qcow2_get_persistent_dirty_bitmap_size

[Kevin Wolf]

Am 08.06.2020 um 21:08 hat Eric Blake geschrieben:
> For now, we don't have persistent bitmaps in any other formats, but
> that might not be true in the future.  Make it obvious that our
> incoming parameter is not necessarily a qcow2 image, and therefore is
> limited to just the bdrv_dirty_bitmap_* API calls (rather than probing
> into qcow2 internals).
> 
> Suggested-by: Kevin Wolf 
> Signed-off-by: Eric Blake 

Thanks, applied to the block branch.

Kevin

Re: [PATCH] block: Refactor subdirectory recursion during make

[Kevin Wolf]

Am 08.06.2020 um 19:33 hat Eric Blake geschrieben:
> Rather than listing block/monitor from the top-level Makefile.objs, we
> should instead list monitor from block/Makefile.objs.
> 
> Suggested-by: Kevin Wolf 
> Fixes: bb4e58c613
> Signed-off-by: Eric Blake 

Thanks, applied to the block branch.

Kevin

Re: [PATCH v6 5/5] iotests: Add test 291 to for qemu-img bitmap coverage

[Kevin Wolf]

Am 21.05.2020 um 21:21 hat Eric Blake geschrieben:
> Add a new test covering the 'qemu-img bitmap' subcommand, as well as
> 'qemu-img convert --bitmaps', both added in recent patches.
> 
> Signed-off-by: Eric Blake 
> Reviewed-by: Max Reitz 
> Reviewed-by: Vladimir Sementsov-Ogievskiy 

This one fails for me. We need to filter out "disk size" because it
depends on the filesystem.

Kevin

291  fail   [21:10:19] [21:10:21]output mismatch 
(see 291.out.bad)
--- /home/kwolf/source/qemu/tests/qemu-iotests/291.out  2020-06-08 
21:04:38.967014949 +0200
+++ /home/kwolf/source/qemu/tests/qemu-iotests/291.out.bad  2020-06-08 
21:10:21.043763527 +0200
@@ -24,7 +24,7 @@
 image: TEST_DIR/t.IMGFMT
 file format: IMGFMT
 virtual size: 10 MiB (10485760 bytes)
-disk size: 4.39 MiB
+disk size: 4.51 MiB
 Format specific information:
 compat: 1.1
 compression type: zlib
@@ -44,7 +44,7 @@
 image: TEST_DIR/t.IMGFMT
 file format: IMGFMT
 virtual size: 10 MiB (10485760 bytes)
-disk size: 4.48 MiB
+disk size: 5.08 MiB
 Format specific information:
 compat: 1.1
 compression type: zlib
Failures: 291
Failed 1 of 1 iotests

[PATCH] qcow2: Tweak comments on qcow2_get_persistent_dirty_bitmap_size

[Eric Blake]

For now, we don't have persistent bitmaps in any other formats, but
that might not be true in the future.  Make it obvious that our
incoming parameter is not necessarily a qcow2 image, and therefore is
limited to just the bdrv_dirty_bitmap_* API calls (rather than probing
into qcow2 internals).

Suggested-by: Kevin Wolf 
Signed-off-by: Eric Blake 
---
 block/qcow2-bitmap.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 7bf12502da8c..1f38806ca6ea 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1757,19 +1757,20 @@ bool 
qcow2_supports_persistent_dirty_bitmap(BlockDriverState *bs)
 }

 /*
- * Compute the space required for bitmaps in @bs.
+ * Compute the space required to copy bitmaps from @in_bs.
  *
  * The computation is based as if copying to a new image with the
- * given @cluster_size, which may differ from the cluster size in @bs.
+ * given @cluster_size, which may differ from the cluster size in
+ * @in_bs; in fact, @in_bs might be something other than qcow2.
  */
-uint64_t qcow2_get_persistent_dirty_bitmap_size(BlockDriverState *bs,
+uint64_t qcow2_get_persistent_dirty_bitmap_size(BlockDriverState *in_bs,
 uint32_t cluster_size)
 {
 uint64_t bitmaps_size = 0;
 BdrvDirtyBitmap *bm;
 size_t bitmap_dir_size = 0;

-FOR_EACH_DIRTY_BITMAP(bs, bm) {
+FOR_EACH_DIRTY_BITMAP(in_bs, bm) {
 if (bdrv_dirty_bitmap_get_persistence(bm)) {
 const char *name = bdrv_dirty_bitmap_name(bm);
 uint32_t granularity = bdrv_dirty_bitmap_granularity(bm);
-- 
2.27.0

Re: [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20200608164357.25065-1-filip.boz...@syrmia.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20200608164357.25065-1-filip.boz...@syrmia.com
Subject: [PATCH v2 0/6] Add strace support for printing arguments of selected 
syscalls
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag] patchew/20200608164357.25065-1-filip.boz...@syrmia.com -> 
patchew/20200608164357.25065-1-filip.boz...@syrmia.com
Switched to a new branch 'test'
320f874 linux-user: Add strace support for printing arguments of fallocate()
a4df4b8 linux-user: Add strace support for printing arguments of 
chown()/lchown()
2a5a74d linux-user: Add strace support for printing arguments of lseek()
ebc81cb linux-user: Add strace support for printing argument of syscalls used 
for extended attributes
595dc96 linux-user: Add strace support for a group of syscalls
b53f9b9 linux-user: Extend strace support to enable argument printing after 
syscall execution

=== OUTPUT BEGIN ===
1/6 Checking commit b53f9b913213 (linux-user: Extend strace support to enable 
argument printing after syscall execution)
2/6 Checking commit 595dc9662da6 (linux-user: Add strace support for a group of 
syscalls)
3/6 Checking commit ebc81cb1d684 (linux-user: Add strace support for printing 
argument of syscalls used for extended attributes)
4/6 Checking commit 2a5a74d1c629 (linux-user: Add strace support for printing 
arguments of lseek())
5/6 Checking commit a4df4b85ae87 (linux-user: Add strace support for printing 
arguments of chown()/lchown())
6/6 Checking commit 320f8741ece4 (linux-user: Add strace support for printing 
arguments of fallocate())
ERROR: storage class should be at the beginning of the declaration
#69: FILE: linux-user/strace.c:1147:
+UNUSED static struct flags falloc_flags[] = {

total: 1 errors, 0 warnings, 104 lines checked

Patch 6/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200608164357.25065-1-filip.boz...@syrmia.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[PATCH v3 7/9] tests/decode: Test non-overlapping groups

[Richard Henderson]

Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 tests/decode/err_pattern_group_nest1.decode  | 14 ++
 tests/decode/err_pattern_group_nest2.decode  |  6 ++
 tests/decode/err_pattern_group_nest3.decode  | 14 ++
 tests/decode/succ_pattern_group_nest3.decode | 11 +++
 4 files changed, 45 insertions(+)
 create mode 100644 tests/decode/err_pattern_group_nest1.decode
 create mode 100644 tests/decode/err_pattern_group_nest2.decode
 create mode 100644 tests/decode/err_pattern_group_nest3.decode
 create mode 100644 tests/decode/succ_pattern_group_nest3.decode

diff --git a/tests/decode/err_pattern_group_nest1.decode 
b/tests/decode/err_pattern_group_nest1.decode
new file mode 100644
index 00..7d09891a1c
--- /dev/null
+++ b/tests/decode/err_pattern_group_nest1.decode
@@ -0,0 +1,14 @@
+# This work is licensed under the terms of the GNU LGPL, version 2 or later.
+# See the COPYING.LIB file in the top-level directory.
+
+%sub1 0:8
+%sub2 8:8
+
+# Make sure braces are matched
+{
+  top   
+  [
+sub1     %sub1
+sub2     %sub1 %sub2
+  }
+}
diff --git a/tests/decode/err_pattern_group_nest2.decode 
b/tests/decode/err_pattern_group_nest2.decode
new file mode 100644
index 00..c172239e9b
--- /dev/null
+++ b/tests/decode/err_pattern_group_nest2.decode
@@ -0,0 +1,6 @@
+# This work is licensed under the terms of the GNU LGPL, version 2 or later.
+# See the COPYING.LIB file in the top-level directory.
+
+# Make sure braces are matched
+{
+  [
diff --git a/tests/decode/err_pattern_group_nest3.decode 
b/tests/decode/err_pattern_group_nest3.decode
new file mode 100644
index 00..b085d01410
--- /dev/null
+++ b/tests/decode/err_pattern_group_nest3.decode
@@ -0,0 +1,14 @@
+# This work is licensed under the terms of the GNU LGPL, version 2 or later.
+# See the COPYING.LIB file in the top-level directory.
+
+%sub1 0:8
+%sub2 8:8
+
+# The exclusive group should error for overlap.
+{
+  top   
+  [
+sub1     %sub1
+sub2     %sub1 %sub2
+  ]
+}
diff --git a/tests/decode/succ_pattern_group_nest3.decode 
b/tests/decode/succ_pattern_group_nest3.decode
new file mode 100644
index 00..156249f090
--- /dev/null
+++ b/tests/decode/succ_pattern_group_nest3.decode
@@ -0,0 +1,11 @@
+# This work is licensed under the terms of the GNU LGPL, version 2 or later.
+# See the COPYING.LIB file in the top-level directory.
+
+{
+  [
+sub1   a:8 b:8 c:8
+sub2  0001 a:8 b:8 c:8
+sub3  0010 a:8 b:8 c:8
+  ]
+  sub400 d:2 a:8 b:8 c:8
+}
-- 
2.25.1

[PATCH v3 9/9] target/arm: Use a non-overlapping group for misc control

[Richard Henderson]

The miscellaneous control instructions are mutually exclusive
within the t32 decode sub-group.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 target/arm/t32.decode | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/t32.decode b/target/arm/t32.decode
index c63082fc9c..c21a988f97 100644
--- a/target/arm/t32.decode
+++ b/target/arm/t32.decode
@@ -312,13 +312,13 @@ CLZ   1010 1011    1000   
@rdm
  &cps
 
 # Miscellaneous control
-{
+[
   CLREX   0011 1011  1000  0010 
   DSB 0011 1011  1000  0100 
   DMB 0011 1011  1000  0101 
   ISB 0011 1011  1000  0110 
   SB  0011 1011  1000  0111 
-}
+]
 
 # Note that the v7m insn overlaps both the normal and banked insn.
 {
-- 
2.25.1

[PATCH v3 6/9] decodetree: Implement non-overlapping groups

[Richard Henderson]

Intended to be nested within overlapping groups.

Reviewed-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Suggested-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 scripts/decodetree.py | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 0ba01e049c..7e3b1d1399 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -1021,16 +1021,20 @@ def parse_file(f, parent_pat):
 del toks[0]
 
 # End nesting?
-if name == '}':
+if name == '}' or name == ']':
 if len(toks) != 0:
 error(start_lineno, 'extra tokens after close brace')
 if len(parent_pat.pats) < 2:
 error(lineno, 'less than two patterns within braces')
 
+# Make sure { } and [ ] nest properly.
+if (name == '}') != isinstance(parent_pat, IncMultiPattern):
+error(lineno, 'mismatched close brace')
+
 try:
 parent_pat = nesting_pats.pop()
 except:
-error(lineno, 'mismatched close brace')
+error(lineno, 'extra close brace')
 
 nesting -= 2
 if indent != nesting:
@@ -1044,11 +1048,14 @@ def parse_file(f, parent_pat):
 error(start_lineno, 'indentation ', indent, ' != ', nesting)
 
 # Start nesting?
-if name == '{':
+if name == '{' or name == '[':
 if len(toks) != 0:
 error(start_lineno, 'extra tokens after open brace')
 
-nested_pat = IncMultiPattern(start_lineno)
+if name == '{':
+nested_pat = IncMultiPattern(start_lineno)
+else:
+nested_pat = ExcMultiPattern(start_lineno)
 parent_pat.pats.append(nested_pat)
 nesting_pats.append(parent_pat)
 parent_pat = nested_pat
@@ -1067,6 +1074,9 @@ def parse_file(f, parent_pat):
 else:
 parse_generic(start_lineno, parent_pat, name, toks)
 toks = []
+
+if nesting != 0:
+error(lineno, 'missing close brace')
 # end parse_file
 
 
-- 
2.25.1

[PATCH v3 8/9] decodetree: Drop check for less than 2 patterns in a group

[Richard Henderson]

While it makes little sense for the end product to have a group
containing only a single pattern, avoiding this case within an
incremental patch set is troublesome.

Because this is expected to be a transient condition, do not
bother "optimizing" this case, e.g. by folding away the group.

Signed-off-by: Richard Henderson 
---
 tests/decode/succ_pattern_group_nest4.decode | 13 +
 scripts/decodetree.py|  2 --
 2 files changed, 13 insertions(+), 2 deletions(-)
 create mode 100644 tests/decode/succ_pattern_group_nest4.decode

diff --git a/tests/decode/succ_pattern_group_nest4.decode 
b/tests/decode/succ_pattern_group_nest4.decode
new file mode 100644
index 00..dc54a1d285
--- /dev/null
+++ b/tests/decode/succ_pattern_group_nest4.decode
@@ -0,0 +1,13 @@
+# This work is licensed under the terms of the GNU LGPL, version 2 or later.
+# See the COPYING.LIB file in the top-level directory.
+
+# Verify deeper nesting, and a single element in the groups.
+{
+  [
+{
+  [
+sub1   a:8 b:8 c:8
+  ]
+}
+  ]
+}
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 7e3b1d1399..530d41ca62 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -1024,8 +1024,6 @@ def parse_file(f, parent_pat):
 if name == '}' or name == ']':
 if len(toks) != 0:
 error(start_lineno, 'extra tokens after close brace')
-if len(parent_pat.pats) < 2:
-error(lineno, 'less than two patterns within braces')
 
 # Make sure { } and [ ] nest properly.
 if (name == '}') != isinstance(parent_pat, IncMultiPattern):
-- 
2.25.1

[PATCH v3 2/9] decodetree: Rename MultiPattern to IncMultiPattern

[Richard Henderson]

Name the current node for "inclusive" multi-pattern, in
preparation for adding a node for "exclusive" multi-pattern.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 scripts/decodetree.py | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index b559db3086..7af6b3056d 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -371,7 +371,7 @@ class Pattern(General):
 # end Pattern
 
 
-class MultiPattern(General):
+class IncMultiPattern(General):
 """Class representing an overlapping set of instruction patterns"""
 
 def __init__(self, lineno, pats, fixb, fixm, udfm, w):
@@ -410,7 +410,7 @@ class MultiPattern(General):
 output(ind, '}\n')
 else:
 p.output_code(i, extracted, p.fixedbits, p.fixedmask)
-#end MultiPattern
+#end IncMultiPattern
 
 
 def parse_field(lineno, name, toks):
@@ -751,8 +751,8 @@ def parse_generic(lineno, is_format, name, toks):
   .format(allbits ^ insnmask))
 # end parse_general
 
-def build_multi_pattern(lineno, pats):
-"""Validate the Patterns going into a MultiPattern."""
+def build_incmulti_pattern(lineno, pats):
+"""Validate the Patterns going into a IncMultiPattern."""
 global patterns
 global insnmask
 
@@ -792,9 +792,9 @@ def build_multi_pattern(lineno, pats):
 else:
 repeat = False
 
-mp = MultiPattern(lineno, pats, fixedbits, fixedmask, undefmask, width)
+mp = IncMultiPattern(lineno, pats, fixedbits, fixedmask, undefmask, width)
 patterns.append(mp)
-# end build_multi_pattern
+# end build_incmulti_pattern
 
 def parse_file(f):
 """Parse all of the patterns within a file"""
@@ -860,7 +860,7 @@ def parse_file(f):
 error(start_lineno, 'indentation ', indent, ' != ', nesting)
 pats = patterns
 patterns = saved_pats.pop()
-build_multi_pattern(lineno, pats)
+build_incmulti_pattern(lineno, pats)
 toks = []
 continue
 
-- 
2.25.1

[PATCH v3 4/9] decodetree: Allow group covering the entire insn space

[Richard Henderson]

This is an edge case for sure, but the logic that disallowed
this case was faulty.  Further, a few fixes scattered about
can allow this to work.

Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 ...est1.decode => succ_pattern_group_nest2.decode} |  2 +-
 scripts/decodetree.py  | 14 +++---
 2 files changed, 12 insertions(+), 4 deletions(-)
 rename tests/decode/{err_pattern_group_nest1.decode => 
succ_pattern_group_nest2.decode} (85%)

diff --git a/tests/decode/err_pattern_group_nest1.decode 
b/tests/decode/succ_pattern_group_nest2.decode
similarity index 85%
rename from tests/decode/err_pattern_group_nest1.decode
rename to tests/decode/succ_pattern_group_nest2.decode
index 92e971c3c5..8d5ab4b2d3 100644
--- a/tests/decode/err_pattern_group_nest1.decode
+++ b/tests/decode/succ_pattern_group_nest2.decode
@@ -6,7 +6,7 @@
 %sub3 16:8
 %sub4 24:8
 
-# Groups with no overlap are supposed to fail
+# Group with complete overlap of the two patterns
 {
   top     
   sub4     %sub1 %sub2 %sub3 %sub4
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index ea313bcdea..3307c74c30 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -124,6 +124,7 @@ def is_pow2(x):
 
 def ctz(x):
 """Return the number of times 2 factors into X."""
+assert x != 0
 r = 0
 while ((x >> r) & 1) == 0:
 r += 1
@@ -131,6 +132,8 @@ def ctz(x):
 
 
 def is_contiguous(bits):
+if bits == 0:
+return -1
 shift = ctz(bits)
 if is_pow2((bits >> shift) + 1):
 return shift
@@ -793,9 +796,8 @@ def build_incmulti_pattern(lineno, pats):
 error(lineno, 'width mismatch in patterns within braces')
 
 repeat = True
-while repeat:
-if fixedmask == 0:
-error(lineno, 'no overlap in patterns within braces')
+fixedbits = 0
+while repeat and fixedmask != 0:
 fixedbits = None
 for p in pats:
 thisbits = p.fixedbits & fixedmask
@@ -978,6 +980,12 @@ def build_tree(pats, outerbits, outermask):
 innermask &= i.fixedmask
 
 if innermask == 0:
+# Edge condition: One pattern covers the entire insnmask
+if len(pats) == 1:
+t = Tree(outermask, innermask)
+t.subs.append((0, pats[0]))
+return t
+
 text = 'overlapping patterns:'
 for p in pats:
 text += '\n' + p.file + ':' + str(p.lineno) + ': ' + str(p)
-- 
2.25.1

[PATCH v3 5/9] decodetree: Move semantic propagation into classes

[Richard Henderson]

Create ExcMultiPattern to hold an set of non-overlapping patterns.
The body of build_tree, prop_format become member functions on this
class.  Add minimal member functions to Pattern and MultiPattern
to allow recusion through the tree.

Move the bulk of build_incmulti_pattern to prop_masks and prop_width
in MultiPattern, since we will need this for both kinds of containers.
Only perform prop_width for variablewidth.

Remove global patterns variable, and pass down container object into
parse_file from main.

No functional change in all of this.

Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 scripts/decodetree.py | 464 +++---
 1 file changed, 253 insertions(+), 211 deletions(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 3307c74c30..0ba01e049c 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -31,7 +31,6 @@ variablewidth = False
 fields = {}
 arguments = {}
 formats = {}
-patterns = []
 allpatterns = []
 anyextern = False
 
@@ -371,16 +370,27 @@ class Pattern(General):
 output(ind, 'u.f_', arg, '.', n, ' = ', f.str_extract(), ';\n')
 output(ind, 'if (', translate_prefix, '_', self.name,
'(ctx, &u.f_', arg, ')) return true;\n')
+
+# Normal patterns do not have children.
+def build_tree(self):
+return
+def prop_masks(self):
+return
+def prop_format(self):
+return
+def prop_width(self):
+return
+
 # end Pattern
 
 
 class MultiPattern(General):
 """Class representing a set of instruction patterns"""
 
-def __init__(self, lineno, pats):
+def __init__(self, lineno):
 self.file = input_file
 self.lineno = lineno
-self.pats = pats
+self.pats = []
 self.base = None
 self.fixedbits = 0
 self.fixedmask = 0
@@ -396,22 +406,63 @@ class MultiPattern(General):
 def output_decl(self):
 for p in self.pats:
 p.output_decl()
+
+def prop_masks(self):
+global insnmask
+
+fixedmask = insnmask
+undefmask = insnmask
+
+# Collect fixedmask/undefmask for all of the children.
+for p in self.pats:
+p.prop_masks()
+fixedmask &= p.fixedmask
+undefmask &= p.undefmask
+
+# Widen fixedmask until all fixedbits match
+repeat = True
+fixedbits = 0
+while repeat and fixedmask != 0:
+fixedbits = None
+for p in self.pats:
+thisbits = p.fixedbits & fixedmask
+if fixedbits is None:
+fixedbits = thisbits
+elif fixedbits != thisbits:
+fixedmask &= ~(fixedbits ^ thisbits)
+break
+else:
+repeat = False
+
+self.fixedbits = fixedbits
+self.fixedmask = fixedmask
+self.undefmask = undefmask
+
+def build_tree(self):
+for p in self.pats:
+p.build_tree()
+
+def prop_format(self):
+for p in self.pats:
+p.build_tree()
+
+def prop_width(self):
+width = None
+for p in self.pats:
+p.prop_width()
+if width is None:
+width = p.width
+elif width != p.width:
+error_with_file(self.file, self.lineno,
+'width mismatch in patterns within braces')
+self.width = width
+
 # end MultiPattern
 
 
 class IncMultiPattern(MultiPattern):
 """Class representing an overlapping set of instruction patterns"""
 
-def __init__(self, lineno, pats, fixb, fixm, udfm, w):
-self.file = input_file
-self.lineno = lineno
-self.pats = pats
-self.base = None
-self.fixedbits = fixb
-self.fixedmask = fixm
-self.undefmask = udfm
-self.width = w
-
 def output_code(self, i, extracted, outerbits, outermask):
 global translate_prefix
 ind = str_indent(i)
@@ -431,6 +482,153 @@ class IncMultiPattern(MultiPattern):
 #end IncMultiPattern
 
 
+class Tree:
+"""Class representing a node in a decode tree"""
+
+def __init__(self, fm, tm):
+self.fixedmask = fm
+self.thismask = tm
+self.subs = []
+self.base = None
+
+def str1(self, i):
+ind = str_indent(i)
+r = '{0}{1:08x}'.format(ind, self.fixedmask)
+if self.format:
+r += ' ' + self.format.name
+r += ' [\n'
+for (b, s) in self.subs:
+r += '{0}  {1:08x}:\n'.format(ind, b)
+r += s.str1(i + 4) + '\n'
+r += ind + ']'
+return r
+
+def __str__(self):
+return self.str1(0)
+
+def output_code(self, i, extracted, outerbits, outermask):
+ind = str_indent(i)
+
+# If we identified all nodes below have the same format,
+# extract the fields now.
+if not extracted and self

[PATCH v3 3/9] decodetree: Split out MultiPattern from IncMultiPattern

[Richard Henderson]

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 scripts/decodetree.py | 37 ++---
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 7af6b3056d..ea313bcdea 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -371,7 +371,32 @@ class Pattern(General):
 # end Pattern
 
 
-class IncMultiPattern(General):
+class MultiPattern(General):
+"""Class representing a set of instruction patterns"""
+
+def __init__(self, lineno, pats):
+self.file = input_file
+self.lineno = lineno
+self.pats = pats
+self.base = None
+self.fixedbits = 0
+self.fixedmask = 0
+self.undefmask = 0
+self.width = None
+
+def __str__(self):
+r = 'group'
+if self.fixedbits is not None:
+r += ' ' + str_match_bits(self.fixedbits, self.fixedmask)
+return r
+
+def output_decl(self):
+for p in self.pats:
+p.output_decl()
+# end MultiPattern
+
+
+class IncMultiPattern(MultiPattern):
 """Class representing an overlapping set of instruction patterns"""
 
 def __init__(self, lineno, pats, fixb, fixm, udfm, w):
@@ -384,16 +409,6 @@ class IncMultiPattern(General):
 self.undefmask = udfm
 self.width = w
 
-def __str__(self):
-r = "{"
-for p in self.pats:
-   r = r + ' ' + str(p)
-return r + "}"
-
-def output_decl(self):
-for p in self.pats:
-p.output_decl()
-
 def output_code(self, i, extracted, outerbits, outermask):
 global translate_prefix
 ind = str_indent(i)
-- 
2.25.1

[PATCH v3 1/9] decodetree: Tidy error_with_file

[Richard Henderson]

Use proper varargs to print the arguments.

Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 scripts/decodetree.py | 24 ++--
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index f9d204aa36..b559db3086 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -51,23 +51,27 @@ def error_with_file(file, lineno, *args):
 global output_file
 global output_fd
 
+prefix = ''
+if file:
+prefix += '{0}:'.format(file)
 if lineno:
-r = '{0}:{1}: error:'.format(file, lineno)
-elif input_file:
-r = '{0}: error:'.format(file)
-else:
-r = 'error:'
-for a in args:
-r += ' ' + str(a)
-r += '\n'
-sys.stderr.write(r)
+prefix += '{0}:'.format(lineno)
+if prefix:
+prefix += ' '
+print(prefix, end='error: ', file=sys.stderr)
+print(*args, file=sys.stderr)
+
 if output_file and output_fd:
 output_fd.close()
 os.remove(output_file)
 exit(1)
+# end error_with_file
+
 
 def error(lineno, *args):
-error_with_file(input_file, lineno, args)
+error_with_file(input_file, lineno, *args)
+# end error
+
 
 def output(*args):
 global output_fd
-- 
2.25.1

[PATCH v3 0/9] decodetree: Add non-overlapping groups

[Richard Henderson]

This is a feature that Peter requested for completing the
neon decodetree conversion.

Changes for v3:
  - Remove check for less than 2 patterns in a group.

Changes for v2:
  - Rebase on master, which includes some generic python cleanups.
  - Indentation error message restored.
  - 4 new testcases

Peter, do you want me to include the final patch in any
decodetree pull request, or shall I leave that for you
to pick up through the target-arm.next tree?


r~


Richard Henderson (9):
  decodetree: Tidy error_with_file
  decodetree: Rename MultiPattern to IncMultiPattern
  decodetree: Split out MultiPattern from IncMultiPattern
  decodetree: Allow group covering the entire insn space
  decodetree: Move semantic propagation into classes
  decodetree: Implement non-overlapping groups
  tests/decode: Test non-overlapping groups
  decodetree: Drop check for less than 2 patterns in a group
  target/arm: Use a non-overlapping group for misc control

 target/arm/t32.decode|   4 +-
 tests/decode/err_pattern_group_nest1.decode  |  11 +-
 tests/decode/err_pattern_group_nest2.decode  |   6 +
 tests/decode/err_pattern_group_nest3.decode  |  14 +
 tests/decode/succ_pattern_group_nest2.decode |  13 +
 tests/decode/succ_pattern_group_nest3.decode |  11 +
 tests/decode/succ_pattern_group_nest4.decode |  13 +
 scripts/decodetree.py| 513 +++
 8 files changed, 360 insertions(+), 225 deletions(-)
 create mode 100644 tests/decode/err_pattern_group_nest2.decode
 create mode 100644 tests/decode/err_pattern_group_nest3.decode
 create mode 100644 tests/decode/succ_pattern_group_nest2.decode
 create mode 100644 tests/decode/succ_pattern_group_nest3.decode
 create mode 100644 tests/decode/succ_pattern_group_nest4.decode

-- 
2.25.1

[PATCH 2/2] block: Call attention to truncation of long NBD exports

[Eric Blake]

Commit 93676c88 relaxed our NBD client code to request export names up
to the NBD protocol maximum of 4096 bytes without NUL terminator, even
though the block layer can't store anything longer than 4096 bytes
including NUL terminator for display to the user.  Since this means
there are some export names where we have to truncate things, we can
at least try to make the truncation a bit more obvious for the user.
Note that in spite of the truncated display name, we can still
communicate with an NBD server using such a long export name; this was
deemed nicer than refusing to even connect to such a server (since the
server may not be under our control, and since determining our actual
length limits gets tricky when nbd://host:port/export and
nbd+unix:///export?socket=/path are themselves variable-length
expansions beyond the export name but count towards the block layer
name length).

Reported-by: Xueqiang Wei 
Fixes: https://bugzilla.redhat.com/1843684
Signed-off-by: Eric Blake 
---
 block.c |  7 +--
 block/nbd.c | 21 +
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/block.c b/block.c
index 8416376c9b71..6dbcb7e083ea 100644
--- a/block.c
+++ b/block.c
@@ -6809,8 +6809,11 @@ void bdrv_refresh_filename(BlockDriverState *bs)
 pstrcpy(bs->filename, sizeof(bs->filename), bs->exact_filename);
 } else {
 QString *json = qobject_to_json(QOBJECT(bs->full_open_options));
-snprintf(bs->filename, sizeof(bs->filename), "json:%s",
- qstring_get_str(json));
+if (snprintf(bs->filename, sizeof(bs->filename), "json:%s",
+ qstring_get_str(json)) >= sizeof(bs->filename)) {
+/* Give user a hint if we truncated things. */
+strcpy(bs->filename + sizeof(bs->filename) - 4, "...");
+}
 qobject_unref(json);
 }
 }
diff --git a/block/nbd.c b/block/nbd.c
index 4ac23c8f6299..eed160c5cda1 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1984,6 +1984,7 @@ static void nbd_refresh_filename(BlockDriverState *bs)
 {
 BDRVNBDState *s = bs->opaque;
 const char *host = NULL, *port = NULL, *path = NULL;
+size_t len = 0;

 if (s->saddr->type == SOCKET_ADDRESS_TYPE_INET) {
 const InetSocketAddress *inet = &s->saddr->u.inet;
@@ -1996,17 +1997,21 @@ static void nbd_refresh_filename(BlockDriverState *bs)
 } /* else can't represent as pseudo-filename */

 if (path && s->export) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd+unix:///%s?socket=%s", s->export, path);
+len = snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+   "nbd+unix:///%s?socket=%s", s->export, path);
 } else if (path && !s->export) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd+unix://?socket=%s", path);
+len = snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+   "nbd+unix://?socket=%s", path);
 } else if (host && s->export) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd://%s:%s/%s", host, port, s->export);
+len = snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+   "nbd://%s:%s/%s", host, port, s->export);
 } else if (host && !s->export) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd://%s:%s", host, port);
+len = snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+   "nbd://%s:%s", host, port);
+}
+if (len > sizeof(bs->exact_filename)) {
+/* Name is too long to represent exactly, so leave it empty. */
+bs->exact_filename[0] = '\0';
 }
 }

-- 
2.27.0

[PATCH 1/2] nbd/server: Avoid long error message assertions CVE-2020-10761

[Eric Blake]

Ever since commit 36683283 (v2.8), the server code asserts that error
strings sent to the client are well-formed per the protocol by not
exceeding the maximum string length of 4096.  At the time the server
first started sending error messages, the assertion could not be
triggered, because messages were completely under our control.
However, over the years, we have added latent scenarios where a client
could trigger the server to attempt an error message that would
include the client's information if it passed other checks first:

- requesting NBD_OPT_INFO/GO on an export name that is not present
  (commit 0cfae925 in v2.12 echoes the name)

- requesting NBD_OPT_LIST/SET_META_CONTEXT on an export name that is
  not present (commit e7b1948d in v2.12 echoes the name)

At the time, those were still safe because we flagged names larger
than 256 bytes with a different message; but that changed in commit
93676c88 (v4.2) when we raised the name limit to 4096 to match the NBD
string limit.  (That commit also failed to change the magic number
4096 in nbd_negotiate_send_rep_err to the just-introduced named
constant.)  So with that commit, long client names appended to server
text can now trigger the assertion, and thus be used as a denial of
service attack against a server.  As a mitigating factor, if the
server requires TLS, the client cannot trigger the problematic paths
unless it first supplies TLS credentials, and such trusted clients are
less likely to try to intentionally crash the server.

Reported-by: Xueqiang Wei 
CC: qemu-sta...@nongnu.org
Fixes: https://bugzilla.redhat.com/1843684 CVE-2020-10761
Fixes: 93676c88d7
Signed-off-by: Eric Blake 
---
 nbd/server.c   | 28 +---
 tests/qemu-iotests/143 |  4 
 tests/qemu-iotests/143.out |  2 ++
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 02b1ed080145..ec130303586d 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -217,7 +217,7 @@ nbd_negotiate_send_rep_verr(NBDClient *client, uint32_t 
type,

 msg = g_strdup_vprintf(fmt, va);
 len = strlen(msg);
-assert(len < 4096);
+assert(len < NBD_MAX_STRING_SIZE);
 trace_nbd_negotiate_send_rep_err(msg);
 ret = nbd_negotiate_send_rep_len(client, type, len, errp);
 if (ret < 0) {
@@ -231,6 +231,27 @@ nbd_negotiate_send_rep_verr(NBDClient *client, uint32_t 
type,
 return 0;
 }

+/*
+ * Truncate a potentially-long user-supplied string into something
+ * more suitable for an error reply.
+ */
+static const char *
+nbd_truncate_name(const char *name)
+{
+#define SANE_LENGTH 80
+static char buf[SANE_LENGTH + 3 + 1]; /* Trailing '...', NUL */
+
+if (strlen(name) < SANE_LENGTH) {
+return name;
+}
+memcpy(buf, name, SANE_LENGTH);
+buf[SANE_LENGTH] = '.';
+buf[SANE_LENGTH + 1] = '.';
+buf[SANE_LENGTH + 2] = '.';
+buf[SANE_LENGTH + 3] = '\0';
+return buf;
+}
+
 /* Send an error reply.
  * Return -errno on error, 0 on success. */
 static int GCC_FMT_ATTR(4, 5)
@@ -597,7 +618,7 @@ static int nbd_negotiate_handle_info(NBDClient *client, 
Error **errp)
 if (!exp) {
 return nbd_negotiate_send_rep_err(client, NBD_REP_ERR_UNKNOWN,
   errp, "export '%s' not present",
-  name);
+  nbd_truncate_name(name));
 }

 /* Don't bother sending NBD_INFO_NAME unless client requested it */
@@ -996,7 +1017,8 @@ static int nbd_negotiate_meta_queries(NBDClient *client,
 meta->exp = nbd_export_find(export_name);
 if (meta->exp == NULL) {
 return nbd_opt_drop(client, NBD_REP_ERR_UNKNOWN, errp,
-"export '%s' not present", export_name);
+"export '%s' not present",
+nbd_truncate_name(export_name));
 }

 ret = nbd_opt_read(client, &nb_queries, sizeof(nb_queries), errp);
diff --git a/tests/qemu-iotests/143 b/tests/qemu-iotests/143
index f649b3619501..b0b1cff86cb6 100755
--- a/tests/qemu-iotests/143
+++ b/tests/qemu-iotests/143
@@ -58,6 +58,10 @@ _send_qemu_cmd $QEMU_HANDLE \
 $QEMU_IO_PROG -f raw -c quit \
 "nbd+unix:///no_such_export?socket=$SOCK_DIR/nbd" 2>&1 \
 | _filter_qemu_io | _filter_nbd
+# Likewise, with longest possible name permitted in NBD protocol
+$QEMU_IO_PROG -f raw -c quit \
+"nbd+unix:///$(printf %4096d 1 | tr ' ' a)?socket=$SOCK_DIR/nbd" 2>&1 \
+| _filter_qemu_io | _filter_nbd | sed 's/aa.*aa/aa...aa/'

 _send_qemu_cmd $QEMU_HANDLE \
 "{ 'execute': 'quit' }" \
diff --git a/tests/qemu-iotests/143.out b/tests/qemu-iotests/143.out
index 1f4001c60131..be1f3a625458 100644
--- a/tests/qemu-iotests/143.out
+++ b/tests/qemu-iotests/143.out
@@ -5,6 +5,8 @@ QA output created by 143
 {"return": {}}
 qemu-io: can't open device nbd+unix:///no_such_export?socket=SOCK_DIR/nbd: 
Requested export not available
 server reported: export 'n

[PATCH 0/2] Fix NBD CVE-2020-10761

[Eric Blake]

In qemu 4.2, I accidentally introduced the ability for an NBD client
obeying the specification to kill qemu as NBD server with an assertion
failure when the client requests an unusually long export name, as a
regression from the intended graceful server error message back to the
client.  Given that the DoS security hole can be mitigated by
requiring TLS (and a client with TLS credentials is less likely to
play such games), the plan is to make the issue public today and send
a pull request through my NBD tree on Tuesday.

We may still want to revisit whether the block layer caps display
names to 4095 bytes, or whether it should track a malloc'd name even
when that name exceeds 4k.

Eric Blake (2):
  nbd/server: Avoid long error message assertions CVE-2020-10761
  block: Call attention to truncation of long NBD exports

 block.c|  7 +--
 block/nbd.c| 21 +
 nbd/server.c   | 28 +---
 tests/qemu-iotests/143 |  4 
 tests/qemu-iotests/143.out |  2 ++
 5 files changed, 49 insertions(+), 13 deletions(-)

-- 
2.27.0

Re: [PATCH] fuzz: add oss-fuzz build.sh script

[Alexander Bulekov]

On 200605 1956, Philippe Mathieu-Daudé wrote:
> On 6/5/20 7:40 PM, Alexander Bulekov wrote:
-cut-
> "make install-datadir"?
I think this just sets up the datadir for subsequent copies:

install-datadir:
$(INSTALL_DIR) "$(DESTDIR)$(qemu_datadir)"

The actual copy happens in install:

install: all $(if $(BUILD_DOCS),install-doc) \
install-datadir install-localstatedir install-includedir 
...
ifneq ($(BLOBS),)
set -e; for x in $(BLOBS); do \
$(INSTALL_DATA) $(SRC_PATH)/pc-bios/$$x 
"$(DESTDIR)$(qemu_datadir)"; \
done
...

> 
> > +
> > +# Copy over the qemu-fuzz-i386, naming it according to each available fuzz
> > +# target (See 05509c8e6d fuzz: select fuzz target using executable name)
> > +for target in $(./i386-softmmu/qemu-fuzz-i386 | awk '$1 ~ /\*/  {print 
> > $2}');
> > +do
> > +cp ./i386-softmmu/qemu-fuzz-i386 $OUT/qemu-fuzz-i386-target-$target
> 
> There seems to be an extra 'target'.
> 
> > +done
> > 
> 
> Or "make install", not sure.

If I can get this to work, hopefully it will also take care of the
datadir.
Thanks
-Alex

Re: [PATCH v3 00/11] hw/sd/sdcard: Fix CVE-2020-13253 & cleanups

[Philippe Mathieu-Daudé]

Hi Alistair,

On 6/5/20 12:22 PM, Philippe Mathieu-Daudé wrote:
> Patches 2 & 3 fix CVE-2020-13253.
> The rest are (accumulated) cleanups.
> 
> Supersedes: <20200604182502.24228-1-f4...@amsat.org>
> 
> Philippe Mathieu-Daudé (11):
>   MAINTAINERS: Cc qemu-block mailing list
>   hw/sd/sdcard: Update coding style to make checkpatch.pl happy
>   hw/sd/sdcard: Do not switch to ReceivingData if address is invalid
>   hw/sd/sdcard: Restrict Class 6 commands to SCSD cards
>   hw/sd/sdcard: Update the SDState documentation
>   hw/sd/sdcard: Simplify cmd_valid_while_locked()
>   hw/sd/sdcard: Constify sd_crc*()'s message argument
>   hw/sd/sdcard: Make iolen unsigned
>   hw/sd/sdcard: Correctly display the command name in trace events
>   hw/sd/sdcard: Display offset in read/write_data() trace events
>   hw/sd/sdcard: Simplify realize() a bit

I forgot to Cc you.

Since you already reviewed a bunch of SD patches in the
past, do you mind having a look a this series? It should
be quite trivial.

Thanks!

Phil.

Re: [PATCH v3 00/16] python: add mypy support to python/qemu

[Philippe Mathieu-Daudé]

On 6/8/20 5:33 PM, Kevin Wolf wrote:
> Am 08.06.2020 um 17:19 hat John Snow geschrieben:
>>
>>
>> On 6/5/20 5:26 AM, Kevin Wolf wrote:
>>> Am 04.06.2020 um 22:22 hat John Snow geschrieben:
 Based-on: 20200604195252.20739-1-js...@redhat.com

 This series is extracted from my larger series that attempted to bundle
 our python module as an installable module. These fixes don't require that,
 so they are being sent first as I think there's less up for debate in here.

 This requires my "refactor shutdown" patch as a pre-requisite.
>>>
>>> You didn't like my previous R-b? Here's a new one. :-)
>>>
>>> Reviewed-by: Kevin Wolf 
>>>
>>
>> I felt like I should address the feedback, and though I could have
>> applied the R-B to patches I didn't change, it was ... faster to just
>> re-send it.
>>
>> Serious question: How do you apply people's R-Bs to your patches? At the
>> moment, it's pretty manually intensive for me. I use stgit and I pop all
>> of the patches off (stg pop -n 100), and then one-at-a-time I `stg push;
>> stg edit` and copy-paste the R-B into it.

wget https://patchew.org/QEMU/${MSG_ID}/mbox
git am mbox

Where ${MSG_ID} is the Message-Id of the series cover letter.

>>
>> It's easier when I'm just pulling other people's patches from the ML,
>> the patches tool handles it for me. It's updating my own patches that's
>> a drag and prone to error.
> 
> It's a manual process for me, too. Just that I don't use stgit, but
> 'git rebase -i'.
> 
> Kevin
> 

Re: [RFC PATCH 15/35] hw/i386/xen/xen-hvm: Emit warning when old code is used

[Philippe Mathieu-Daudé]

On 6/8/20 6:54 PM, Paul Durrant wrote:
>> -Original Message-
>> From: Philippe Mathieu-Daudé 
>>
>> This code hasn't been QOM'ified yet. Warn the user.
> 
> "Based on today's IRC chat, this is a trivial RFC series
> to anotate pre-qdev/QOM devices so developers using them
> without knowing they are not QOM'ified yet can realize
> it and convert them if they have time."
> 
> So, how should this be coded then? The XenIOState doesn't really qualify as a 
> 'device', does it?

There is a pending question whether Machines are Devices or not.

Xen is a tricky case, it is created as a Q35 machine overloaded with Xen
features.

>> @@ -1401,6 +1401,8 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion 
>> **ram_memory)
>>  xen_pfn_t ioreq_pfn;
>>  XenIOState *state;
>>
>> +qdev_warn_deprecated_function_used();
>> +
>>  state = g_malloc0(sizeof (XenIOState));

XenIOState is indeed not a device, it is allocated once, we won't gain
anything making it a QOM object... so this patch is incorrect.

Sorry for the noise :S

Regards,

Phil.

>>
>>  state->xce_handle = xenevtchn_open(NULL, 0);
>> --
>> 2.21.3
> 
> 

[PATCH] block: Refactor subdirectory recursion during make

[Eric Blake]

Rather than listing block/monitor from the top-level Makefile.objs, we
should instead list monitor from block/Makefile.objs.

Suggested-by: Kevin Wolf 
Fixes: bb4e58c613
Signed-off-by: Eric Blake 
---
 Makefile.objs   | 2 +-
 block/Makefile.objs | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile.objs b/Makefile.objs
index 99774cfd2545..0c08eb863a14 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -13,7 +13,7 @@ chardev-obj-y = chardev/

 authz-obj-y = authz/

-block-obj-y = block/ block/monitor/ nbd/ scsi/
+block-obj-y = block/ nbd/ scsi/
 block-obj-y += block.o blockjob.o job.o
 block-obj-y += qemu-io-cmds.o
 block-obj-$(CONFIG_REPLICATION) += replication.o
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 87357529f339..5947da08575f 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -47,6 +47,7 @@ block-obj-y += aio_task.o
 block-obj-y += backup-top.o
 block-obj-y += filter-compress.o
 common-obj-y += monitor/
+block-obj-y += monitor/

 block-obj-y += stream.o

-- 
2.27.0

Re: [PATCH] linux-user/sparc64: Translate flushw opcode

[Philippe Mathieu-Daudé]

Hello Giuseppe,

On 4/10/20 11:14 PM, LemonBoy wrote:
> From 11d0cfe58d12e0f191b435ade88622cfceb2098a Mon Sep 17 00:00:00 2001
> From: LemonBoy 
> Date: Fri, 10 Apr 2020 22:55:26 +0200
> Subject: [PATCH] linux-user/sparc64: Translate flushw opcode
> 
> The ifdef logic should unconditionally compile in the `xop == 0x2b` case
> when targeting sparc64.
> 
> Fix the handling of window spill traps by keeping cansave into account
> when calculating the new CWP.

Thanks for the fix.

Do you mind addressing the comment Richard suggested?

Thanks!

Phil.

> 
> Signed-off-by: Giuseppe Musacchio 
> ---
>  bsd-user/main.c | 4 +++-
>  linux-user/sparc/cpu_loop.c | 4 +++-
>  target/sparc/translate.c| 2 ++
>  3 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 770c2b267a..d6b1c997e3 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -413,7 +413,9 @@ static void save_window(CPUSPARCState *env)
>  save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2));
>  env->wim = new_wim;
>  #else
> -save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2));
> +/* cansave is zero if the spill trap handler is triggered by `save` and 
> */
> +/* nonzero if triggered by a `flushw` */
> +save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2));
>  env->cansave++;
>  env->canrestore--;
>  #endif
> diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c
> index 7645cc04ca..20a7401126 100644
> --- a/linux-user/sparc/cpu_loop.c
> +++ b/linux-user/sparc/cpu_loop.c
> @@ -69,7 +69,9 @@ static void save_window(CPUSPARCState *env)
>  save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2));
>  env->wim = new_wim;
>  #else
> -save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2));
> +/* cansave is zero if the spill trap handler is triggered by `save` and 
> */
> +/* nonzero if triggered by a `flushw` */
> +save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2));
>  env->cansave++;
>  env->canrestore--;
>  #endif
> diff --git a/target/sparc/translate.c b/target/sparc/translate.c
> index 9416a551cf..1a4efd4ed6 100644
> --- a/target/sparc/translate.c
> +++ b/target/sparc/translate.c
> @@ -3663,6 +3663,8 @@ static void disas_sparc_insn(DisasContext * dc, 
> unsigned int insn)
>  #endif
>  gen_store_gpr(dc, rd, cpu_tmp0);
>  break;
> +#endif
> +#if defined(TARGET_SPARC64) || !defined(CONFIG_USER_ONLY)
>  } else if (xop == 0x2b) { /* rdtbr / V9 flushw */
>  #ifdef TARGET_SPARC64
>  gen_helper_flushw(cpu_env);
> 

Re: [PATCH 1/3] hw/sparc/leon3: Map the UART device unconditionally

[Fred Konrad]

Le 6/8/20 à 7:21 PM, Philippe Mathieu-Daudé a écrit :

The UART is present on the chipset regardless there is a
character device connected to it. Map it unconditionally.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/sparc/leon3.c | 18 --
  1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 8f024dab7b..cc55117dec 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -339,16 +339,14 @@ static void leon3_generic_hw_init(MachineState *machine)
  0, LEON3_TIMER_IRQ, GRLIB_APBIO_AREA);
  
  /* Allocate uart */

-if (serial_hd(0)) {
-dev = qdev_create(NULL, TYPE_GRLIB_APB_UART);
-qdev_prop_set_chr(dev, "chrdev", serial_hd(0));
-qdev_init_nofail(dev);
-sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_UART_OFFSET);
-sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, cpu_irqs[LEON3_UART_IRQ]);
-grlib_apb_pnp_add_entry(apb_pnp, LEON3_UART_OFFSET, 0xFFF,
-GRLIB_VENDOR_GAISLER, GRLIB_APBUART_DEV, 1,
-LEON3_UART_IRQ, GRLIB_APBIO_AREA);
-}
+dev = qdev_create(NULL, TYPE_GRLIB_APB_UART);
+qdev_prop_set_chr(dev, "chrdev", serial_hd(0));
+qdev_init_nofail(dev);
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_UART_OFFSET);
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, cpu_irqs[LEON3_UART_IRQ]);
+grlib_apb_pnp_add_entry(apb_pnp, LEON3_UART_OFFSET, 0xFFF,
+GRLIB_VENDOR_GAISLER, GRLIB_APBUART_DEV, 1,
+LEON3_UART_IRQ, GRLIB_APBIO_AREA);
  }
  
  static void leon3_generic_machine_init(MachineClass *mc)




Reviewed-by: KONRAD Frederic 

Thanks!

[PATCH 1/3] hw/sparc/leon3: Map the UART device unconditionally

[Philippe Mathieu-Daudé]

The UART is present on the chipset regardless there is a
character device connected to it. Map it unconditionally.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/sparc/leon3.c | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 8f024dab7b..cc55117dec 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -339,16 +339,14 @@ static void leon3_generic_hw_init(MachineState *machine)
 0, LEON3_TIMER_IRQ, GRLIB_APBIO_AREA);
 
 /* Allocate uart */
-if (serial_hd(0)) {
-dev = qdev_create(NULL, TYPE_GRLIB_APB_UART);
-qdev_prop_set_chr(dev, "chrdev", serial_hd(0));
-qdev_init_nofail(dev);
-sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_UART_OFFSET);
-sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, cpu_irqs[LEON3_UART_IRQ]);
-grlib_apb_pnp_add_entry(apb_pnp, LEON3_UART_OFFSET, 0xFFF,
-GRLIB_VENDOR_GAISLER, GRLIB_APBUART_DEV, 1,
-LEON3_UART_IRQ, GRLIB_APBIO_AREA);
-}
+dev = qdev_create(NULL, TYPE_GRLIB_APB_UART);
+qdev_prop_set_chr(dev, "chrdev", serial_hd(0));
+qdev_init_nofail(dev);
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, LEON3_UART_OFFSET);
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, cpu_irqs[LEON3_UART_IRQ]);
+grlib_apb_pnp_add_entry(apb_pnp, LEON3_UART_OFFSET, 0xFFF,
+GRLIB_VENDOR_GAISLER, GRLIB_APBUART_DEV, 1,
+LEON3_UART_IRQ, GRLIB_APBIO_AREA);
 }
 
 static void leon3_generic_machine_init(MachineClass *mc)
-- 
2.21.3

[PATCH 2/3] hw/sparc64/niagara: Remove duplicated NIAGARA_UART_BASE definition

[Philippe Mathieu-Daudé]

NIAGARA_UART_BASE is already defined few lines earlier.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/sparc64/niagara.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/sparc64/niagara.c b/hw/sparc64/niagara.c
index ab5ef8c5b3..201fb05d50 100644
--- a/hw/sparc64/niagara.c
+++ b/hw/sparc64/niagara.c
@@ -68,7 +68,6 @@ typedef struct NiagaraBoardState {
 
 #define NIAGARA_VDISK_BASE  0x1f4000ULL
 #define NIAGARA_RTC_BASE0xfff0c1fff8ULL
-#define NIAGARA_UART_BASE   0x1f1000ULL
 
 /* Firmware layout
  *
-- 
2.21.3

[PATCH 0/3] hw/sparc: Map the UART devices unconditionally

[Philippe Mathieu-Daudé]

Few more SPARC patches.

Mark/Artyom/Frederic if you Ack them I'll simply add them
to the current trivial SPARC patch queue I prepared.

Regards,

Phil.

Philippe Mathieu-Daudé (3):
  hw/sparc/leon3: Map the UART device unconditionally
  hw/sparc64/niagara: Remove duplicated NIAGARA_UART_BASE definition
  hw/sparc64/niagara: Map the UART device unconditionally

 hw/sparc/leon3.c | 18 --
 hw/sparc64/niagara.c |  7 ++-
 2 files changed, 10 insertions(+), 15 deletions(-)

-- 
2.21.3

[PATCH 3/3] hw/sparc64/niagara: Map the UART device unconditionally

[Philippe Mathieu-Daudé]

The UART is present on the machine regardless there is a
character device connected to it. Map it unconditionally.

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

diff --git a/hw/sparc64/niagara.c b/hw/sparc64/niagara.c
index 201fb05d50..a87d55f6bb 100644
--- a/hw/sparc64/niagara.c
+++ b/hw/sparc64/niagara.c
@@ -151,10 +151,8 @@ static void niagara_init(MachineState *machine)
 exit(1);
 }
 }
-if (serial_hd(0)) {
-serial_mm_init(sysmem, NIAGARA_UART_BASE, 0, NULL, 115200,
-   serial_hd(0), DEVICE_BIG_ENDIAN);
-}
+serial_mm_init(sysmem, NIAGARA_UART_BASE, 0, NULL,
+   115200, serial_hd(0), DEVICE_BIG_ENDIAN);
 create_unimplemented_device("sun4v-iob", NIAGARA_IOBBASE, NIAGARA_IOBSIZE);
 sun4v_rtc_init(NIAGARA_RTC_BASE);
 }
-- 
2.21.3

[PATCH v2 6/6] target/i386: reimplement fprem, fprem1 using floatx80 operations

[Joseph Myers]

The x87 fprem and fprem1 emulation is currently based around
conversion to double, which is inherently unsuitable for a good
emulation of any floatx80 operation.  Reimplement using the soft-float
floatx80 remainder operations.

Signed-off-by: Joseph Myers 
Reviewed-by: Richard Henderson 
---
 target/i386/fpu_helper.c | 156 ---
 1 file changed, 48 insertions(+), 108 deletions(-)

diff --git a/target/i386/fpu_helper.c b/target/i386/fpu_helper.c
index 8ef5b463ea..0e531e3821 100644
--- a/target/i386/fpu_helper.c
+++ b/target/i386/fpu_helper.c
@@ -934,124 +934,64 @@ void helper_fxtract(CPUX86State *env)
 merge_exception_flags(env, old_flags);
 }
 
-void helper_fprem1(CPUX86State *env)
+static void helper_fprem_common(CPUX86State *env, bool mod)
 {
-double st0, st1, dblq, fpsrcop, fptemp;
-CPU_LDoubleU fpsrcop1, fptemp1;
-int expdif;
-signed long long int q;
-
-st0 = floatx80_to_double(env, ST0);
-st1 = floatx80_to_double(env, ST1);
-
-if (isinf(st0) || isnan(st0) || isnan(st1) || (st1 == 0.0)) {
-ST0 = double_to_floatx80(env, 0.0 / 0.0); /* NaN */
-env->fpus &= ~0x4700; /* (C3,C2,C1,C0) <--  */
-return;
-}
-
-fpsrcop = st0;
-fptemp = st1;
-fpsrcop1.d = ST0;
-fptemp1.d = ST1;
-expdif = EXPD(fpsrcop1) - EXPD(fptemp1);
-
-if (expdif < 0) {
-/* optimisation? taken from the AMD docs */
-env->fpus &= ~0x4700; /* (C3,C2,C1,C0) <--  */
-/* ST0 is unchanged */
-return;
-}
+uint8_t old_flags = save_exception_flags(env);
+uint64_t quotient;
+CPU_LDoubleU temp0, temp1;
+int exp0, exp1, expdiff;
 
-if (expdif < 53) {
-dblq = fpsrcop / fptemp;
-/* round dblq towards nearest integer */
-dblq = rint(dblq);
-st0 = fpsrcop - fptemp * dblq;
+temp0.d = ST0;
+temp1.d = ST1;
+exp0 = EXPD(temp0);
+exp1 = EXPD(temp1);
 
-/* convert dblq to q by truncating towards zero */
-if (dblq < 0.0) {
-q = (signed long long int)(-dblq);
+env->fpus &= ~0x4700; /* (C3,C2,C1,C0) <--  */
+if (floatx80_is_zero(ST0) || floatx80_is_zero(ST1) ||
+exp0 == 0x7fff || exp1 == 0x7fff ||
+floatx80_invalid_encoding(ST0) || floatx80_invalid_encoding(ST1)) {
+ST0 = floatx80_modrem(ST0, ST1, mod, "ient, &env->fp_status);
+} else {
+if (exp0 == 0) {
+exp0 = 1 - clz64(temp0.l.lower);
+}
+if (exp1 == 0) {
+exp1 = 1 - clz64(temp1.l.lower);
+}
+expdiff = exp0 - exp1;
+if (expdiff < 64) {
+ST0 = floatx80_modrem(ST0, ST1, mod, "ient, &env->fp_status);
+env->fpus |= (quotient & 0x4) << (8 - 2);  /* (C0) <-- q2 */
+env->fpus |= (quotient & 0x2) << (14 - 1); /* (C3) <-- q1 */
+env->fpus |= (quotient & 0x1) << (9 - 0);  /* (C1) <-- q0 */
 } else {
-q = (signed long long int)dblq;
+/*
+ * Partial remainder.  This choice of how many bits to
+ * process at once is specified in AMD instruction set
+ * manuals, and empirically is followed by Intel
+ * processors as well; it ensures that the final remainder
+ * operation in a loop does produce the correct low three
+ * bits of the quotient.  AMD manuals specify that the
+ * flags other than C2 are cleared, and empirically Intel
+ * processors clear them as well.
+ */
+int n = 32 + (expdiff % 32);
+temp1.d = floatx80_scalbn(temp1.d, expdiff - n, &env->fp_status);
+ST0 = floatx80_mod(ST0, temp1.d, &env->fp_status);
+env->fpus |= 0x400;  /* C2 <-- 1 */
 }
-
-env->fpus &= ~0x4700; /* (C3,C2,C1,C0) <--  */
-/* (C0,C3,C1) <-- (q2,q1,q0) */
-env->fpus |= (q & 0x4) << (8 - 2);  /* (C0) <-- q2 */
-env->fpus |= (q & 0x2) << (14 - 1); /* (C3) <-- q1 */
-env->fpus |= (q & 0x1) << (9 - 0);  /* (C1) <-- q0 */
-} else {
-env->fpus |= 0x400;  /* C2 <-- 1 */
-fptemp = pow(2.0, expdif - 50);
-fpsrcop = (st0 / st1) / fptemp;
-/* fpsrcop = integer obtained by chopping */
-fpsrcop = (fpsrcop < 0.0) ?
-  -(floor(fabs(fpsrcop))) : floor(fpsrcop);
-st0 -= (st1 * fpsrcop * fptemp);
 }
-ST0 = double_to_floatx80(env, st0);
+merge_exception_flags(env, old_flags);
 }
 
-void helper_fprem(CPUX86State *env)
+void helper_fprem1(CPUX86State *env)
 {
-double st0, st1, dblq, fpsrcop, fptemp;
-CPU_LDoubleU fpsrcop1, fptemp1;
-int expdif;
-signed long long int q;
-
-st0 = floatx80_to_double(env, ST0);
-st1 = floatx80_to_double(env, ST1);
-
-if (isinf(st0) || isnan(st0) || isnan(st1) || (st1 == 0.0)) {
-ST0 = double_to_floatx80(env, 0.0 / 0.0); /* NaN */
-env->fpus &= ~0x4700; /* (C3,C2,C1,C0) <-- 0

Re: [PATCH v2 1/1] virtio-ccw: auto-manage VIRTIO_F_IOMMU_PLATFORM if PV

[Halil Pasic]

[..]
> > Let me list some pros and cons (compared to the previous patch):
> > 
> > PRO:
> > * Thanks to on/off/auto we don't override what the user specified. From 
> > user interface perspective preferable. I usually hate software that
> > thinks its than me and can do the opposite I tell it.
> 
> Agreed.
> 
> > 
> > CON:
> > * It is more code: "4 files changed, 37 insertions(+), 2 deletions(-)"
> >   against "3 files changed, 17 insertions(+)"
> > * Unlike the previous one, this one is not fool-proof! The user can
> >   still specify access_platform=off to lets say a hotplug device, and
> >   bring down the guest. We could however fence such stuff with an error
> >   message. Would be even more code though.
> 
> I think trying to hotplug such a device to a guest running in protected
> mode should simply fail (and not crash anything.)

Yes, if on/off/auto with a similar semantics like here is the path
we are going to walk, I will definitely have to throw in some code that
fails the hotplug of such devices.

> 
> > * As far as I can tell 'auto' was used to pick a value on initialization
> >   time. This is a novel, and possibly dodgy use in a sense that the value
> >   of the property may change during the lifetime of the VM.
> 
> You mean that we start the vm once with support for prot virt, and
> later without?

No, this patch does not care if VM supports prot virt or not, it only
cares about the mode we are running in (prot virt or not). That is, I
still might add F_ACCESS_PLATFORM when the VM gets transitioned to a
prot virt VM. And this is something other uses of OnOffAuto don't seem
to do. 

> 
> > * We may need to do something about libvirt.
> 
> I'm also not 100% sure about migration... would it make sense to
> discuss all of this in the context of the cross-arch patchset? It seems
> power has similar issues.
> 

I'm going to definitely have a good look at that. What I think special
about s390 is that F_ACCESS_PLATFORM is hurting us because all IO needs
to go through ZONE_DMA (this is a problem of the implementation that
stemming form a limitation of the DMA API, upstream didn't let me
fix it). 

> > 
> > Further improvements are possible and probably necessary if we want
> > to go down this path. But I would like to verify that first.
> > 
> > 8<-
> > From: Halil Pasic 
> > Date: Wed, 26 Feb 2020 16:48:21 +0100
> > Subject: [PATCH v2.5 1/1] virtio-ccw: auto-manage VIRTIO_F_IOMMU_PLATFORM 
> > if PV
> > 
> > The virtio specification tells that the device is to present
> > VIRTIO_F_ACCESS_PLATFORM (a.k.a. VIRTIO_F_IOMMU_PLATFORM) when the
> > device "can only access certain memory addresses with said access
> > specified and/or granted by the platform". This is the case for a
> > protected VMs, as the device can access only memory addresses that are
> > in pages that are currently shared (only the guest can share/unsare its
> > pages).
> > 
> > No VM, however, starts out as a protected VM, but some VMs may be
> > converted to protected VMs if the guest decides so.
> > 
> > Making the end user explicitly manage the VIRTIO_F_ACCESS_PLATFORM via
> > the property iommu_on is a minor disaster. Since the correctness of the
> > paravirtualized virtio devices depends (and thus in a sense the
> > correctness of the hypervisor) it, then the hypervisor should have the
> > last word about whether VIRTIO_F_ACCESS_PLATFORM is to be presented or
> > not.
> > 
> > Currently presenting a PV guest with a (paravirtualized) virtio-ccw
> > device has catastrophic consequences for the VM (after the hypervisors
> > access to protected memory). This is especially grave in case of device
> > hotplug (because in this case the guest is more likely to be in the
> > middle of something important).
> 
> You mean for virtio-ccw devices that don't have iommu_on, right? 
> 
> 

Right, I'm missing the most important words.

> > 
> > Let us add the ability to manage the VIRTIO_F_ACCESS_PLATFORM virtio
> > feature automatically. This is accomplished  by turning the property
> > into an 'on/off/auto' property, and for virtio-ccw devices if auto
> > was specified forcing its value before  we start the protected VM. If
> > the VM should cease to be protected, the original value is restored.
> > 
> > Signed-off-by: Halil Pasic 
> > ---
> >  hw/s390x/s390-virtio-ccw.c |  2 ++
> >  hw/s390x/virtio-ccw.c  | 14 ++
> >  hw/virtio/virtio.c | 19 +++
> >  include/hw/virtio/virtio.h |  4 ++--
> >  4 files changed, 37 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> > index f660070d22..705e6b153a 100644
> > --- a/hw/s390x/s390-virtio-ccw.c
> > +++ b/hw/s390x/s390-virtio-ccw.c
> > @@ -330,6 +330,7 @@ static void s390_machine_unprotect(S390CcwMachineState 
> > *ms)
> >  migrate_del_blocker(pv_mig_blocker);
> >  error_free_or_abort(&pv_mig_blocker);
> >  qemu_balloon_inhibit(false)

[PATCH v2 5/6] softfloat: return low bits of quotient from floatx80_modrem

[Joseph Myers]

Both x87 and m68k need the low parts of the quotient for their
remainder operations.  Arrange for floatx80_modrem to track those bits
and return them via a pointer.

The architectures using float32_rem and float64_rem do not appear to
need this information, so the *_rem interface is left unchanged and
the information returned only from floatx80_modrem.  The logic used to
determine the low 7 bits of the quotient for m68k
(target/m68k/fpu_helper.c:make_quotient) appears completely bogus (it
looks at the result of converting the remainder to integer, the
quotient having been discarded by that point); this patch does not
change that, but the m68k maintainers may wish to do so.

Signed-off-by: Joseph Myers 
Reviewed-by: Richard Henderson 
---
 fpu/softfloat.c | 23 ++-
 include/fpu/softfloat.h |  3 ++-
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 1552241b5e..72f45b0103 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -5684,10 +5684,11 @@ floatx80 floatx80_div(floatx80 a, floatx80 b, 
float_status *status)
 | `a' with respect to the corresponding value `b'.  The operation is performed
 | according to the IEC/IEEE Standard for Binary Floating-Point Arithmetic,
 | if 'mod' is false; if 'mod' is true, return the remainder based on truncating
-| the quotient toward zero instead.
+| the quotient toward zero instead.  '*quotient' is set to the low 64 bits of
+| the absolute value of the integer quotient.
 **/
 
-floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
+floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod, uint64_t *quotient,
  float_status *status)
 {
 bool aSign, zSign;
@@ -5695,6 +5696,7 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
 uint64_t aSig0, aSig1, bSig;
 uint64_t q, term0, term1, alternateASig0, alternateASig1;
 
+*quotient = 0;
 if (floatx80_invalid_encoding(a) || floatx80_invalid_encoding(b)) {
 float_raise(float_flag_invalid, status);
 return floatx80_default_nan(status);
@@ -5753,7 +5755,7 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
 shift128Right( aSig0, 0, 1, &aSig0, &aSig1 );
 expDiff = 0;
 }
-q = ( bSig <= aSig0 );
+*quotient = q = ( bSig <= aSig0 );
 if ( q ) aSig0 -= bSig;
 expDiff -= 64;
 while ( 0 < expDiff ) {
@@ -5763,6 +5765,8 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
 sub128( aSig0, aSig1, term0, term1, &aSig0, &aSig1 );
 shortShift128Left( aSig0, aSig1, 62, &aSig0, &aSig1 );
 expDiff -= 62;
+*quotient <<= 62;
+*quotient += q;
 }
 expDiff += 64;
 if ( 0 < expDiff ) {
@@ -5776,6 +5780,12 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool 
mod,
 ++q;
 sub128( aSig0, aSig1, term0, term1, &aSig0, &aSig1 );
 }
+if (expDiff < 64) {
+*quotient <<= expDiff;
+} else {
+*quotient = 0;
+}
+*quotient += q;
 }
 else {
 term1 = 0;
@@ -5790,6 +5800,7 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
 aSig0 = alternateASig0;
 aSig1 = alternateASig1;
 zSign = ! zSign;
+++*quotient;
 }
 }
 return
@@ -5806,7 +5817,8 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
 
 floatx80 floatx80_rem(floatx80 a, floatx80 b, float_status *status)
 {
-return floatx80_modrem(a, b, false, status);
+uint64_t quotient;
+return floatx80_modrem(a, b, false, "ient, status);
 }
 
 /*
@@ -5817,7 +5829,8 @@ floatx80 floatx80_rem(floatx80 a, floatx80 b, 
float_status *status)
 
 floatx80 floatx80_mod(floatx80 a, floatx80 b, float_status *status)
 {
-return floatx80_modrem(a, b, true, status);
+uint64_t quotient;
+return floatx80_modrem(a, b, true, "ient, status);
 }
 
 /*
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index bff6934d09..ff4e2605b1 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -687,7 +687,8 @@ floatx80 floatx80_add(floatx80, floatx80, float_status 
*status);
 floatx80 floatx80_sub(floatx80, floatx80, float_status *status);
 floatx80 floatx80_mul(floatx80, floatx80, float_status *status);
 floatx80 floatx80_div(floatx80, floatx80, float_status *status);
-floatx80 floatx80_modrem(floatx80, floatx80, bool, float_status *status);
+floatx80 floatx80_modrem(floatx80, floatx80, bool, uint64_t *,
+ float_status *status);
 floatx80 floatx80_mod(floatx80, floatx80, float_status *status);
 floatx80 floatx80_rem(floatx80, floatx80, float_status *status);
 floatx80 floatx80_sqrt(floatx80, float_status *status);

[PATCH v2 3/6] softfloat: do not return pseudo-denormal from floatx80 remainder

[Joseph Myers]

The floatx80 remainder implementation sometimes returns the numerator
unchanged when the denominator is sufficiently larger than the
numerator.  But if the value to be returned unchanged is a
pseudo-denormal, that is incorrect.  Fix it to normalize the numerator
in that case.

Signed-off-by: Joseph Myers 
Reviewed-by: Richard Henderson 
---
 fpu/softfloat.c | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 091847beb9..9d43868e4c 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -5691,7 +5691,7 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
  float_status *status)
 {
 bool aSign, zSign;
-int32_t aExp, bExp, expDiff;
+int32_t aExp, bExp, expDiff, aExpOrig;
 uint64_t aSig0, aSig1, bSig;
 uint64_t q, term0, term1, alternateASig0, alternateASig1;
 
@@ -5700,7 +5700,7 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
 return floatx80_default_nan(status);
 }
 aSig0 = extractFloatx80Frac( a );
-aExp = extractFloatx80Exp( a );
+aExpOrig = aExp = extractFloatx80Exp( a );
 aSign = extractFloatx80Sign( a );
 bSig = extractFloatx80Frac( b );
 bExp = extractFloatx80Exp( b );
@@ -5715,6 +5715,13 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool 
mod,
 if ((uint64_t)(bSig << 1)) {
 return propagateFloatx80NaN(a, b, status);
 }
+if (aExp == 0 && aSig0 >> 63) {
+/*
+ * Pseudo-denormal argument must be returned in normalized
+ * form.
+ */
+return packFloatx80(aSign, 1, aSig0);
+}
 return a;
 }
 if ( bExp == 0 ) {
@@ -5734,7 +5741,16 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool 
mod,
 expDiff = aExp - bExp;
 aSig1 = 0;
 if ( expDiff < 0 ) {
-if ( mod || expDiff < -1 ) return a;
+if ( mod || expDiff < -1 ) {
+if (aExp == 1 && aExpOrig == 0) {
+/*
+ * Pseudo-denormal argument must be returned in
+ * normalized form.
+ */
+return packFloatx80(aSign, aExp, aSig0);
+}
+return a;
+}
 shift128Right( aSig0, 0, 1, &aSig0, &aSig1 );
 expDiff = 0;
 }
-- 
2.17.1


-- 
Joseph S. Myers
jos...@codesourcery.com

[PATCH v2 0/6] Add strace support for printing arguments of selected syscalls

[Filip Bozuta]

From: Filip Bozuta 

This series covers strace support for printing arguments of following syscalls:

*acct()   *lgetxattr()   *removexattr()   *lchown()
*fsync()  *fgetxattr()   *lremovexattr()  *fallocate()
*fdatasync()  *listxattr()   *fremovexattr()
*listen() *llistxattr()  *lseek()
*getxattr()   *flistxattr()  *chown()

The implementation details for strace support is described in this series patch
commit messages.

Testing method:

Mini test programs were written that run these syscalls for different 
arguments.
Those programs were compiled (sometimes using cross-compilers) for the 
following
architectures:

* Intel 64-bit (little endian) (gcc)
* Power pc 32-bit (big endian) (powerpc-linux-gnu-gcc)
* Power pc 64-bit (big endian) (powerpc64-linux-gnu-gcc)
* Mips 32-bit (little endian) (mipsel-linux-gnu-gcc)
* Mips 64-bit (little endian) (mips64el-linux-gnuabi64-gcc)

The corresponding native programs were executed with strace, without using
QEMU, on Intel Core i7-4790K (x86_64) host.

All applicable compiled programs were in turn executed with "-strace"
through QEMU and the strace printing results obtained were the same 
ones gotten for native execution.

v2:

* Added patch that extends strace support by enabling argument printing
  after syscall execution
* Added strace support for argument printing for syscalls:
  removexattr(), lremovexattr(), fremovexattr()
* Added "print_syscall_ret_listxattr()" that prints list of extended
  attributes after execution of syscalls: listxattr(), llistxattr(),
  flistxattr()
* Corrected formats in some printing functions
* Moved target_offset64() function definition from "syscall.c" to
  "qemu.h"

Filip Bozuta (6):
  linux-user: Extend strace support to enable argument printing after
syscall execution
  linux-user: Add strace support for a group of syscalls
  linux-user: Add strace support for printing argument of syscalls used
for extended attributes
  linux-user: Add strace support for printing arguments of lseek()
  linux-user: Add strace support for printing arguments of
chown()/lchown()
  linux-user: Add strace support for printing arguments of fallocate()

 linux-user/qemu.h  |  20 +++-
 linux-user/strace.c| 247 -
 linux-user/strace.list |  37 +++---
 linux-user/syscall.c   |  18 +--
 4 files changed, 281 insertions(+), 41 deletions(-)

-- 
2.17.1

[PATCH v2 4/6] softfloat: do not set denominator high bit for floatx80 remainder

[Joseph Myers]

The floatx80 remainder implementation unnecessarily sets the high bit
of bSig explicitly.  By that point in the function, arguments that are
invalid, zero, infinity or NaN have already been handled and
subnormals have been through normalizeFloatx80Subnormal, so the high
bit will already be set.  Remove the unnecessary code.

Signed-off-by: Joseph Myers 
Reviewed-by: Richard Henderson 
---
 fpu/softfloat.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 9d43868e4c..1552241b5e 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -5736,7 +5736,6 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
 if ( aSig0 == 0 ) return a;
 normalizeFloatx80Subnormal( aSig0, &aExp, &aSig0 );
 }
-bSig |= UINT64_C(0x8000);
 zSign = aSign;
 expDiff = aExp - bExp;
 aSig1 = 0;
-- 
2.17.1


-- 
Joseph S. Myers
jos...@codesourcery.com

[PATCH v2 1/6] softfloat: merge floatx80_mod and floatx80_rem

[Joseph Myers]

The m68k-specific softfloat code includes a function floatx80_mod that
is extremely similar to floatx80_rem, but computing the remainder
based on truncating the quotient toward zero rather than rounding it
to nearest integer.  This is also useful for emulating the x87 fprem
and fprem1 instructions.  Change the floatx80_rem implementation into
floatx80_modrem that can perform either operation, with both
floatx80_rem and floatx80_mod as thin wrappers available for all
targets.

There does not appear to be any use for the _mod operation for other
floating-point formats in QEMU (the only other architectures using
_rem at all are linux-user/arm/nwfpe, for FPA emulation, and openrisc,
for instructions that have been removed in the latest version of the
architecture), so no change is made to the code for other formats.

Signed-off-by: Joseph Myers 
Reviewed-by: Richard Henderson 
---
 fpu/softfloat.c | 49 ++--
 include/fpu/softfloat.h |  2 +
 target/m68k/softfloat.c | 83 -
 target/m68k/softfloat.h |  1 -
 4 files changed, 40 insertions(+), 95 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 6c8f2d597a..7b1ce7664f 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -5682,10 +5682,13 @@ floatx80 floatx80_div(floatx80 a, floatx80 b, 
float_status *status)
 /*
 | Returns the remainder of the extended double-precision floating-point value
 | `a' with respect to the corresponding value `b'.  The operation is performed
-| according to the IEC/IEEE Standard for Binary Floating-Point Arithmetic.
+| according to the IEC/IEEE Standard for Binary Floating-Point Arithmetic,
+| if 'mod' is false; if 'mod' is true, return the remainder based on truncating
+| the quotient toward zero instead.
 **/
 
-floatx80 floatx80_rem(floatx80 a, floatx80 b, float_status *status)
+floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
+ float_status *status)
 {
 bool aSign, zSign;
 int32_t aExp, bExp, expDiff;
@@ -5731,7 +5734,7 @@ floatx80 floatx80_rem(floatx80 a, floatx80 b, 
float_status *status)
 expDiff = aExp - bExp;
 aSig1 = 0;
 if ( expDiff < 0 ) {
-if ( expDiff < -1 ) return a;
+if ( mod || expDiff < -1 ) return a;
 shift128Right( aSig0, 0, 1, &aSig0, &aSig1 );
 expDiff = 0;
 }
@@ -5763,14 +5766,16 @@ floatx80 floatx80_rem(floatx80 a, floatx80 b, 
float_status *status)
 term1 = 0;
 term0 = bSig;
 }
-sub128( term0, term1, aSig0, aSig1, &alternateASig0, &alternateASig1 );
-if (lt128( alternateASig0, alternateASig1, aSig0, aSig1 )
- || (eq128( alternateASig0, alternateASig1, aSig0, aSig1 )
-  && ( q & 1 ) )
-   ) {
-aSig0 = alternateASig0;
-aSig1 = alternateASig1;
-zSign = ! zSign;
+if (!mod) {
+sub128( term0, term1, aSig0, aSig1, &alternateASig0, &alternateASig1 );
+if (lt128( alternateASig0, alternateASig1, aSig0, aSig1 )
+|| (eq128( alternateASig0, alternateASig1, aSig0, aSig1 )
+&& ( q & 1 ) )
+) {
+aSig0 = alternateASig0;
+aSig1 = alternateASig1;
+zSign = ! zSign;
+}
 }
 return
 normalizeRoundAndPackFloatx80(
@@ -5778,6 +5783,28 @@ floatx80 floatx80_rem(floatx80 a, floatx80 b, 
float_status *status)
 
 }
 
+/*
+| Returns the remainder of the extended double-precision floating-point value
+| `a' with respect to the corresponding value `b'.  The operation is performed
+| according to the IEC/IEEE Standard for Binary Floating-Point Arithmetic.
+**/
+
+floatx80 floatx80_rem(floatx80 a, floatx80 b, float_status *status)
+{
+return floatx80_modrem(a, b, false, status);
+}
+
+/*
+| Returns the remainder of the extended double-precision floating-point value
+| `a' with respect to the corresponding value `b', with the quotient truncated
+| toward zero.
+**/
+
+floatx80 floatx80_mod(floatx80 a, floatx80 b, float_status *status)
+{
+return floatx80_modrem(a, b, true, status);
+}
+
 /*
 | Returns the square root of the extended double-precision floating-point
 | value `a'.  The operation is performed according to the IEC/IEEE Standard
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 16ca697a73..bff6934d09 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -687,6 +687,8 @@ floatx80 float

[PATCH v2 2/6] softfloat: fix floatx80 remainder pseudo-denormal check for zero

[Joseph Myers]

The floatx80 remainder implementation ignores the high bit of the
significand when checking whether an operand (numerator) with zero
exponent is zero.  This means it mishandles a pseudo-denormal
representation of 0x1p-16382L by treating it as zero.  Fix this by
checking the whole significand instead.

Signed-off-by: Joseph Myers 
Reviewed-by: Richard Henderson 
---
 fpu/softfloat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 7b1ce7664f..091847beb9 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -5726,7 +5726,7 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
 normalizeFloatx80Subnormal( bSig, &bExp, &bSig );
 }
 if ( aExp == 0 ) {
-if ( (uint64_t) ( aSig0<<1 ) == 0 ) return a;
+if ( aSig0 == 0 ) return a;
 normalizeFloatx80Subnormal( aSig0, &aExp, &aSig0 );
 }
 bSig |= UINT64_C(0x8000);
-- 
2.17.1


-- 
Joseph S. Myers
jos...@codesourcery.com

[PATCH v2 0/6] softfloat, target/i386: fprem, fprem1 fixes

[Joseph Myers]

The x87 floating-point emulation of the fprem and fprem1 instructions
works via conversion to and from double.  This is inherently
unsuitable for a good emulation of any floatx80 operation.  This patch
series adapts the softfloat floatx80_rem implementation to be suitable
for these instructions and uses it to reimplement them.

There is an existing test for these instructions, test-i386-fprem.c,
based on comparison of output.  It produces 1679695 lines of output,
and before this patch series 415422 of those lines are different on
hardware from the output produced by QEMU.  Some of those differences
are because QEMU's x87 emulation does not yet produce the "denormal
operand" exception; ignoring such differences (modifying the output
from a native run not to report that exception), there are still
398833 different lines.  This patch series reduces that latter number
to 1 (that one difference being because of missing checks for
floating-point stack underflow, another global issue with the x87
emulation), or 35517 different lines without the correction for lack
of denormal operand exception support.

Several fixes to and new features in the softfloat support for this
operation are needed; floatx80_mod, previously present in the m68k
code only, is made generic and unified with floatx80_rem in a new
floatx80_modrem of which floatx80_mod and floatx80_rem are thin
wrappers.  The only architectures using float*_rem for other formats
are arm (FPA emulation) and openrisc (instructions that have been
removed in the latest architecture version); they do not appear to
need any of the new features, and all the bugs fixed are specific to
floatx80, so no changes are made to the remainder implementation for
those formats.

A new feature added is returning the low bits of the quotient from
floatx80_modrem, as needed for both x87 and m68k.  The logic used to
determine the low 7 bits of the quotient for m68k
(target/m68k/fpu_helper.c:make_quotient) appears completely bogus (it
looks at the result of converting the remainder to integer, the
quotient having been discarded by that point); this patch series does
not change that to use the new interface, but the m68k maintainers may
wish to do so.

The Intel instruction set documentation leaves unspecified the exact
number of bits by which the remainder instructions reduce the operand
each time.  The AMD documentation gives a specific formula, which
empirically Intel processors follow as well, and that formula is
implemented in the code.  The AMD documentation also specifies that
flags other than C2 are cleared in the partial remainder case, whereas
the Intel manual is silent on that (but the processors do appear to
clear those flags); this patch implements that flag clearing, and
keeps the existing flag clearing in cases where the instructions raise
"invalid" (although it seems hardware in fact only clears some but not
all flags in that case, leaving other flags unchanged).

The Intel manuals include an inaccurate table asserting that (finite
REM 0) should raise "divide by zero"; actually, in accordance with
IEEE semantics, it raises "invalid".  The AMD manuals inaccurately say
for both fprem and fprem1 that if the exponent difference is negative,
the numerator is returned unchanged, which is correct (apart from
normalizing pseudo-denormals) for fprem but not for fprem1 (and the
old QEMU code had an incorrect optimization following the AMD manuals
for fprem1).

Changes in version 2 of the patch series: fix comment formatting and
combine patches 6 and 7.

Joseph Myers (6):
  softfloat: merge floatx80_mod and floatx80_rem
  softfloat: fix floatx80 remainder pseudo-denormal check for zero
  softfloat: do not return pseudo-denormal from floatx80 remainder
  softfloat: do not set denominator high bit for floatx80 remainder
  softfloat: return low bits of quotient from floatx80_modrem
  target/i386: reimplement fprem, fprem1 using floatx80 operations

 fpu/softfloat.c  |  87 ++
 include/fpu/softfloat.h  |   3 +
 target/i386/fpu_helper.c | 156 ---
 target/m68k/softfloat.c  |  83 -
 target/m68k/softfloat.h  |   1 -
 5 files changed, 122 insertions(+), 208 deletions(-)

-- 
2.17.1


-- 
Joseph S. Myers
jos...@codesourcery.com

RE: [RFC PATCH 15/35] hw/i386/xen/xen-hvm: Emit warning when old code is used

[Paul Durrant]

> -Original Message-
> From: Philippe Mathieu-Daudé 
> Sent: 08 June 2020 17:00
> To: qemu-devel@nongnu.org
> Cc: qemu-...@nongnu.org; Markus Armbruster ; Max Filippov 
> ;
> Marcel Apfelbaum ; Peter Maydell 
> ; Michael Walle
> ; Edgar E. Iglesias ; Aurelien 
> Jarno
> ; Gerd Hoffmann ; Stafford Horne 
> ; Andrzej
> Zaborowski ; qemu-...@nongnu.org; Alistair Francis 
> ;
> Richard Henderson ; Mark Cave-Ayland 
> ; Marc-André
> Lureau ; Daniel P . Berrange 
> ; qemu-
> ri...@nongnu.org; Michael S. Tsirkin ; 
> xen-de...@lists.xenproject.org; Sagar
> Karandikar ; Anthony Perard 
> ; Palmer Dabbelt
> ; Stefano Stabellini ; Paul 
> Durrant ; Paolo
> Bonzini ; Alistair Francis ; 
> Eduardo Habkost
> ; Thomas Huth ; Bastian Koppelmann 
>  paderborn.de>; David Gibson ; Magnus Damm 
> ;
> Philippe Mathieu-Daudé 
> Subject: [RFC PATCH 15/35] hw/i386/xen/xen-hvm: Emit warning when old code is 
> used
> 
> This code hasn't been QOM'ified yet. Warn the user.

"Based on today's IRC chat, this is a trivial RFC series
to anotate pre-qdev/QOM devices so developers using them
without knowing they are not QOM'ified yet can realize
it and convert them if they have time."

So, how should this be coded then? The XenIOState doesn't really qualify as a 
'device', does it?

  Paul

> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/i386/xen/xen-hvm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
> index 82ece6b9e7..a1163b1529 100644
> --- a/hw/i386/xen/xen-hvm.c
> +++ b/hw/i386/xen/xen-hvm.c
> @@ -31,7 +31,7 @@
>  #include "sysemu/xen-mapcache.h"
>  #include "trace.h"
>  #include "exec/address-spaces.h"
> -
> +#include "hw/qdev-deprecated.h"
>  #include 
>  #include 
> 
> @@ -1401,6 +1401,8 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion 
> **ram_memory)
>  xen_pfn_t ioreq_pfn;
>  XenIOState *state;
> 
> +qdev_warn_deprecated_function_used();
> +
>  state = g_malloc0(sizeof (XenIOState));
> 
>  state->xce_handle = xenevtchn_open(NULL, 0);
> --
> 2.21.3

Re: [PATCH 6/7] target/i386: reimplement fprem1 using floatx80 operations

[Joseph Myers]

On Mon, 8 Jun 2020, Alex Bennée wrote:

> > +uint8_t old_flags = save_exception_flags(env);
> 
> Hmm where did this come from:

This series assumes all my other recent x87 fixes (11 such patches in 
three series that aren't yet on master, there's also a single patch for 
pcmpxstrx which is independent of those) are already present.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH v2 1/1] virtio-ccw: auto-manage VIRTIO_F_IOMMU_PLATFORM if PV

[Michael S. Tsirkin]

On Sat, Jun 06, 2020 at 01:32:17AM +0200, Halil Pasic wrote:
> On Wed, 20 May 2020 12:23:24 -0400
> "Michael S. Tsirkin"  wrote:
> 
> > On Fri, May 15, 2020 at 12:11:55AM +0200, Halil Pasic wrote:
> > > The virtio specification tells that the device is to present
> > > VIRTIO_F_ACCESS_PLATFORM (a.k.a. VIRTIO_F_IOMMU_PLATFORM) when the
> > > device "can only access certain memory addresses with said access
> > > specified and/or granted by the platform". This is the case for a
> > > protected VMs, as the device can access only memory addresses that are
> > > in pages that are currently shared (only the guest can share/unsare its
> > > pages).
> > > 
> > > No VM, however, starts out as a protected VM, but some VMs may be
> > > converted to protected VMs if the guest decides so.
> > > 
> > > Making the end user explicitly manage the VIRTIO_F_ACCESS_PLATFORM via
> > > the property iommu_on is a minor disaster. Since the correctness of the
> > > paravirtualized virtio devices depends (and thus in a sense the
> > > correctness of the hypervisor) it, then the hypervisor should have the
> > > last word about whether VIRTIO_F_ACCESS_PLATFORM is to be presented or
> > > not.
> > 
> > So, how about this: switch iommu to on/off/auto.  Add a property with a
> > reasonable name "allow protected"?  If set allow switch to protected
> > memory and also set iommu auto to on by default.  If not set then don't.
> > 
> > This will come handy for other things like migrating to hosts without
> > protected memory support.
> > 
> > 
> > Also, virtio now calls this PLATFORM_ACCESS, maybe we should rename
> > the property (keeping old one around for compat)?
> > I feel this will address lots of complaints ...
> > 
> 
> I'm not sure I've entirely understood your proposal, but I tried to
> do something in that direction. 
> 
> Short summary of the changes: 
> * added new property "access_platform" as on/off/auto (default auto)
> * added alias "iommu_platform" for compatibility
> * rewrote this patch to on/off/auto so that we only do the override when
>   user specified auto 
> 
> Let me list some pros and cons (compared to the previous patch):
> 
> PRO:
> * Thanks to on/off/auto we don't override what the user specified. From 
> user interface perspective preferable. I usually hate software that
> thinks its than me and can do the opposite I tell it.
> 
> CON:
> * It is more code: "4 files changed, 37 insertions(+), 2 deletions(-)"
>   against "3 files changed, 17 insertions(+)"
> * Unlike the previous one, this one is not fool-proof! The user can
>   still specify access_platform=off to lets say a hotplug device, and
>   bring down the guest. We could however fence such stuff with an error
>   message. Would be even more code though.
> * As far as I can tell 'auto' was used to pick a value on initialization
>   time. This is a novel, and possibly dodgy use in a sense that the value
>   of the property may change during the lifetime of the VM.
> * We may need to do something about libvirt.
> 
> Further improvements are possible and probably necessary if we want
> to go down this path. But I would like to verify that first.

I think it should be even simpler. If switching to protected
VM is *allowed* by host then auto should mean on.
No changes of features across reset necessary.



> 8<-
> From: Halil Pasic 
> Date: Wed, 26 Feb 2020 16:48:21 +0100
> Subject: [PATCH v2.5 1/1] virtio-ccw: auto-manage VIRTIO_F_IOMMU_PLATFORM if 
> PV
> 
> The virtio specification tells that the device is to present
> VIRTIO_F_ACCESS_PLATFORM (a.k.a. VIRTIO_F_IOMMU_PLATFORM) when the
> device "can only access certain memory addresses with said access
> specified and/or granted by the platform". This is the case for a
> protected VMs, as the device can access only memory addresses that are
> in pages that are currently shared (only the guest can share/unsare its
> pages).
> 
> No VM, however, starts out as a protected VM, but some VMs may be
> converted to protected VMs if the guest decides so.
> 
> Making the end user explicitly manage the VIRTIO_F_ACCESS_PLATFORM via
> the property iommu_on is a minor disaster. Since the correctness of the
> paravirtualized virtio devices depends (and thus in a sense the
> correctness of the hypervisor) it, then the hypervisor should have the
> last word about whether VIRTIO_F_ACCESS_PLATFORM is to be presented or
> not.
> 
> Currently presenting a PV guest with a (paravirtualized) virtio-ccw
> device has catastrophic consequences for the VM (after the hypervisors
> access to protected memory). This is especially grave in case of device
> hotplug (because in this case the guest is more likely to be in the
> middle of something important).
> 
> Let us add the ability to manage the VIRTIO_F_ACCESS_PLATFORM virtio
> feature automatically. This is accomplished  by turning the property
> into an 'on/off/auto' property, and for virtio-ccw devices if auto
> was specifi

Re: [PATCH 6/7] target/i386: reimplement fprem1 using floatx80 operations

[Alex Bennée]

Joseph Myers  writes:

> The x87 fprem1 emulation is currently based around conversion to
> double, which is inherently unsuitable for a good emulation of any
> floatx80 operation.  Reimplement using the soft-float floatx80
> remainder operations.
>
> Signed-off-by: Joseph Myers 
> ---
>  target/i386/fpu_helper.c | 96 +++-
>  1 file changed, 45 insertions(+), 51 deletions(-)
>
> diff --git a/target/i386/fpu_helper.c b/target/i386/fpu_helper.c
> index 8ef5b463ea..bab35e00a0 100644
> --- a/target/i386/fpu_helper.c
> +++ b/target/i386/fpu_helper.c
> @@ -934,63 +934,57 @@ void helper_fxtract(CPUX86State *env)
>  merge_exception_flags(env, old_flags);
>  }
>  
> -void helper_fprem1(CPUX86State *env)
> +static void helper_fprem_common(CPUX86State *env, bool mod)
>  {
> -double st0, st1, dblq, fpsrcop, fptemp;
> -CPU_LDoubleU fpsrcop1, fptemp1;
> -int expdif;
> -signed long long int q;
> -
> -st0 = floatx80_to_double(env, ST0);
> -st1 = floatx80_to_double(env, ST1);
> -
> -if (isinf(st0) || isnan(st0) || isnan(st1) || (st1 == 0.0)) {
> -ST0 = double_to_floatx80(env, 0.0 / 0.0); /* NaN */
> -env->fpus &= ~0x4700; /* (C3,C2,C1,C0) <--  */
> -return;
> -}
> -
> -fpsrcop = st0;
> -fptemp = st1;
> -fpsrcop1.d = ST0;
> -fptemp1.d = ST1;
> -expdif = EXPD(fpsrcop1) - EXPD(fptemp1);
> -
> -if (expdif < 0) {
> -/* optimisation? taken from the AMD docs */
> -env->fpus &= ~0x4700; /* (C3,C2,C1,C0) <--  */
> -/* ST0 is unchanged */
> -return;
> -}
> +uint8_t old_flags = save_exception_flags(env);

Hmm where did this come from:

/home/alex/lsrc/qemu.git/target/i386/fpu_helper.c: In function 
‘helper_fprem_common’:
/home/alex/lsrc/qemu.git/target/i386/fpu_helper.c:784:25: error: implicit 
declaration of function ‘save_exception_flags’; did you mean 
‘raise_exception_ra’? [-Werror=implicit-function-declaration]
 uint8_t old_flags = save_exception_flags(env);
 ^~~~
 raise_exception_ra
/home/alex/lsrc/qemu.git/target/i386/fpu_helper.c:784:25: error: nested extern 
declaration of ‘save_exception_flags’ [-Werror=nested-externs]
/home/alex/lsrc/qemu.git/target/i386/fpu_helper.c:827:5: error: implicit 
declaration of function ‘merge_exception_flags’; did you mean 
‘get_float_exception_flags’? [-Werror=implicit-function-declaration]
 merge_exception_flags(env, old_flags);
 ^
 get_float_exception_flags
/home/alex/lsrc/qemu.git/target/i386/fpu_helper.c:827:5: error: nested extern 
declaration of ‘merge_exception_flags’ [-Werror=nested-externs]
cc1: all warnings being treated as errors

-- 
Alex Bennée

[PATCH v2 6/6] linux-user: Add strace support for printing arguments of fallocate()

[Filip Bozuta]

From: Filip Bozuta 

This patch implements strace argument printing functionality for following 
syscall:

*fallocate - manipulate file space

int fallocate(int fd, int mode, off_t offset, off_t len)
man page: https://www.man7.org/linux/man-pages/man2/fallocate.2.html

Implementation notes:

This syscall's second argument "mode" is composed of predefined values
which represent flags that determine the type of operation that is
to be performed on the file space. For that reason, a printing
function "print_fallocate" was stated in file "strace.list". This printing
function uses an already existing function "print_flags()" to print flags of
the "mode" argument. These flags are stated inside an array "falloc_flags"
that contains values of type "struct flags". These values are instantiated
using an existing macro "FLAG_GENERIC()". Most of these flags are defined
after kernel version 3.0 which is why they are enwrapped in an #ifdef
directive.
The syscall's third ant fourth argument are of type "off_t" which can
cause variations between 32/64-bit architectures. To handle this variation,
function "target_offset64()" was copied from file "strace.c" and used in
"print_fallocate" to print "off_t" arguments for 32-bit architectures.

Signed-off-by: Filip Bozuta 
---
 linux-user/qemu.h  | 16 
 linux-user/strace.c| 40 
 linux-user/strace.list |  2 +-
 linux-user/syscall.c   | 16 
 4 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 8f938b8105..be67391ba4 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -670,6 +670,22 @@ static inline int is_error(abi_long ret)
 return (abi_ulong)ret >= (abi_ulong)(-4096);
 }
 
+#if TARGET_ABI_BITS == 32
+static inline uint64_t target_offset64(uint32_t word0, uint32_t word1)
+{
+#ifdef TARGET_WORDS_BIGENDIAN
+return ((uint64_t)word0 << 32) | word1;
+#else
+return ((uint64_t)word1 << 32) | word0;
+#endif
+}
+#else /* TARGET_ABI_BITS == 32 */
+static inline uint64_t target_offset64(uint64_t word0, uint64_t word1)
+{
+return word0;
+}
+#endif /* TARGET_ABI_BITS != 32 */
+
 /**
  * preexit_cleanup: housekeeping before the guest exits
  *
diff --git a/linux-user/strace.c b/linux-user/strace.c
index cd6a2b8e3f..3ef7f80cd7 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1144,6 +1144,26 @@ UNUSED static struct flags statx_mask[] = {
 FLAG_END,
 };
 
+UNUSED static struct flags falloc_flags[] = {
+FLAG_GENERIC(FALLOC_FL_KEEP_SIZE),
+FLAG_GENERIC(FALLOC_FL_PUNCH_HOLE),
+#ifdef FALLOC_FL_NO_HIDE_STALE
+FLAG_GENERIC(FALLOC_FL_NO_HIDE_STALE),
+#endif
+#ifdef FALLOC_FL_COLLAPSE_RANGE
+FLAG_GENERIC(FALLOC_FL_COLLAPSE_RANGE),
+#endif
+#ifdef FALLOC_FL_ZERO_RANGE
+FLAG_GENERIC(FALLOC_FL_ZERO_RANGE),
+#endif
+#ifdef FALLOC_FL_INSERT_RANGE
+FLAG_GENERIC(FALLOC_FL_INSERT_RANGE),
+#endif
+#ifdef FALLOC_FL_UNSHARE_RANGE
+FLAG_GENERIC(FALLOC_FL_UNSHARE_RANGE),
+#endif
+};
+
 /*
  * print_xxx utility functions.  These are used to print syscall
  * parameters in certain format.  All of these have parameter
@@ -1561,6 +1581,26 @@ print_faccessat(const struct syscallname *name,
 }
 #endif
 
+#ifdef TARGET_NR_fallocate
+static void
+print_fallocate(const struct syscallname *name,
+abi_long arg0, abi_long arg1, abi_long arg2,
+abi_long arg3, abi_long arg4, abi_long arg5)
+{
+print_syscall_prologue(name);
+print_raw_param("%d", arg0, 0);
+print_flags(falloc_flags, arg1, 0);
+#if TARGET_ABI_BITS == 32
+print_raw_param("%" PRIu64, target_offset64(arg2, arg3), 0);
+print_raw_param("%" PRIu64, target_offset64(arg4, arg5), 1);
+#else
+print_raw_param(TARGET_ABI_FMT_ld, arg2, 0);
+print_raw_param(TARGET_ABI_FMT_ld, arg3, 1);
+#endif
+print_syscall_epilogue(name);
+}
+#endif
+
 #ifdef TARGET_NR_fchmodat
 static void
 print_fchmodat(const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index 44eb515ca4..a8d7cbe7a4 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -182,7 +182,7 @@
 { TARGET_NR_fadvise64_64, "fadvise64_64" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_fallocate
-{ TARGET_NR_fallocate, "fallocate" , NULL, NULL, NULL },
+{ TARGET_NR_fallocate, "fallocate" , NULL, print_fallocate, NULL },
 #endif
 #ifdef TARGET_NR_fanotify_init
 { TARGET_NR_fanotify_init, "fanotify_init" , NULL, NULL, NULL },
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 009bb67422..7cc5a65b4f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6608,22 +6608,6 @@ void syscall_init(void)
 }
 }
 
-#if TARGET_ABI_BITS == 32
-static inline uint64_t target_offset64(uint32_t word0, uint32_t word1)
-{
-#ifdef TARGET_WORDS_BIGENDIAN
-return ((uint64_t)word0 << 32) | word1;
-#else
-return ((uint64_t)word1 << 32) | word0;
-#endif
-}
-#els

[PATCH v2 4/6] linux-user: Add strace support for printing arguments of lseek()

[Filip Bozuta]

From: Filip Bozuta 

This patch implements strace argument printing functionality for syscall:

*lseek - reposition read/write file offset

 off_t lseek(int fd, off_t offset, int whence)
 man page: https://www.man7.org/linux/man-pages/man2/lseek.2.html

Implementation notes:

The syscall's third argument "whence" has predefined values:
"SEEK_SET","SEEK_CUR","SEEK_END","SEEK_DATA","SEEK_HOLE"
and thus a separate printing function "print_lseek" was stated
in file "strace.list". This function is defined in "strace.c"
by using an existing function "print_raw_param()" to print
the first and second argument and a switch(case) statement
for the predefined values of the third argument.
Values "SEEK_DATA" and "SEEK_HOLE" are defined in kernel version 3.1.
That is the reason why case statements for these values are
enwrapped in #ifdef directive.

Signed-off-by: Filip Bozuta 
---
 linux-user/strace.c| 31 +++
 linux-user/strace.list |  2 +-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 59fdb0a05f..adf9a47465 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1842,6 +1842,37 @@ print__llseek(const struct syscallname *name,
 }
 #endif
 
+#ifdef TARGET_NR_lseek
+static void
+print_lseek(const struct syscallname *name,
+abi_long arg0, abi_long arg1, abi_long arg2,
+abi_long arg3, abi_long arg4, abi_long arg5)
+{
+print_syscall_prologue(name);
+print_raw_param("%d", arg0, 0);
+print_raw_param(TARGET_ABI_FMT_ld, arg1, 0);
+switch (arg2) {
+case SEEK_SET:
+qemu_log("SEEK_SET"); break;
+case SEEK_CUR:
+qemu_log("SEEK_CUR"); break;
+case SEEK_END:
+qemu_log("SEEK_END"); break;
+#ifdef SEEK_DATA
+case SEEK_DATA:
+qemu_log("SEEK_DATA"); break;
+#endif
+#ifdef SEEK_HOLE
+case SEEK_HOLE:
+qemu_log("SEEK_HOLE"); break;
+#endif
+default:
+print_raw_param("%#x", arg2, 1);
+}
+print_syscall_epilogue(name);
+}
+#endif
+
 #if defined(TARGET_NR_socket)
 static void
 print_socket(const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index 05a72370c1..bd04596c50 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -516,7 +516,7 @@
 { TARGET_NR_lremovexattr, "lremovexattr" , NULL, print_lremovexattr, NULL },
 #endif
 #ifdef TARGET_NR_lseek
-{ TARGET_NR_lseek, "lseek" , NULL, NULL, NULL },
+{ TARGET_NR_lseek, "lseek" , NULL, print_lseek, NULL },
 #endif
 #ifdef TARGET_NR_lsetxattr
 { TARGET_NR_lsetxattr, "lsetxattr" , NULL, NULL, NULL },
-- 
2.17.1

[PATCH v2 5/6] linux-user: Add strace support for printing arguments of chown()/lchown()

[Filip Bozuta]

From: Filip Bozuta 

This patch implements strace argument printing functionality for syscalls:

*chown, lchown - change ownership of a file

int chown(const char *pathname, uid_t owner, gid_t group)
int lchown(const char *pathname, uid_t owner, gid_t group)
man page: https://www.man7.org/linux/man-pages/man2/lchown.2.html

Implementation notes:

Both syscalls use strings as arguments and thus a separate
printing function was stated in "strace.list" for them.
Both syscalls share the same number and types of arguments
and thus share a same definition in file "syscall.c".
This defintion uses existing functions "print_string()" to
print the string argument and "print_raw_param()" to print
other two arguments that are of basic types.

Signed-off-by: Filip Bozuta 
Reviewed-by: Laurent Vivier 
---
 linux-user/strace.c| 15 +++
 linux-user/strace.list |  4 ++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index adf9a47465..cd6a2b8e3f 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1461,6 +1461,21 @@ print_chmod(const struct syscallname *name,
 }
 #endif
 
+#if defined(TARGET_NR_chown) || defined(TARGET_NR_lchown)
+static void
+print_chown(const struct syscallname *name,
+abi_long arg0, abi_long arg1, abi_long arg2,
+abi_long arg3, abi_long arg4, abi_long arg5)
+{
+print_syscall_prologue(name);
+print_string(arg0, 0);
+print_raw_param("%d", arg1, 0);
+print_raw_param("%d", arg2, 1);
+print_syscall_epilogue(name);
+}
+#define print_lchown print_chown
+#endif
+
 #ifdef TARGET_NR_clock_adjtime
 static void
 print_clock_adjtime(const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index bd04596c50..44eb515ca4 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -71,7 +71,7 @@
 { TARGET_NR_chmod, "chmod" , NULL, print_chmod, NULL },
 #endif
 #ifdef TARGET_NR_chown
-{ TARGET_NR_chown, "chown" , NULL, NULL, NULL },
+{ TARGET_NR_chown, "chown" , NULL, print_chown, NULL },
 #endif
 #ifdef TARGET_NR_chown32
 { TARGET_NR_chown32, "chown32" , NULL, NULL, NULL },
@@ -475,7 +475,7 @@
 { TARGET_NR_kill, "kill", NULL, print_kill, NULL },
 #endif
 #ifdef TARGET_NR_lchown
-{ TARGET_NR_lchown, "lchown" , NULL, NULL, NULL },
+{ TARGET_NR_lchown, "lchown" , NULL, print_lchown, NULL },
 #endif
 #ifdef TARGET_NR_lchown32
 { TARGET_NR_lchown32, "lchown32" , NULL, NULL, NULL },
-- 
2.17.1

[PATCH v2 3/6] linux-user: Add strace support for printing argument of syscalls used for extended attributes

[Filip Bozuta]

From: Filip Bozuta 

This patch implements strace argument printing functionality for following 
syscalls:

*getxattr, lgetxattr, fgetxattr - retrieve an extended attribute value

ssize_t getxattr(const char *path, const char *name, void *value, 
size_t size)
ssize_t lgetxattr(const char *path, const char *name, void *value, 
size_t size)
ssize_t fgetxattr(int fd, const char *name, void *value, size_t size)
man page: https://www.man7.org/linux/man-pages/man2/getxattr.2.html

*listxattr, llistxattr, flistxattr - list extended attribute names

ssize_t listxattr(const char *path, char *list, size_t size)
ssize_t llistxattr(const char *path, char *list, size_t size)
ssize_t flistxattr(int fd, char *list, size_t size)
man page: https://www.man7.org/linux/man-pages/man2/listxattr.2.html

*removexattr, lremovexattr, fremovexattr - remove an extended attribute

 int removexattr(const char *path, const char *name)
 int lremovexattr(const char *path, const char *name)
 int fremovexattr(int fd, const char *name)
 man page: https://www.man7.org/linux/man-pages/man2/removexattr.2.html

Implementation notes:

All of the syscalls have strings as argument types and thus a separate
printing function was stated in file "strace.list" for every one of them.
All of these printing functions were defined in "strace.c" using existing
printing functions for appropriate argument types:
   "print_string()" - for (const char*) type
   "print_pointer()" - for (char*) and (void *) type
   "print_raw_param()" for (int) and (size_t) type
Syscalls "getxattr()" and "lgetxattr()" have the same number and type of
arguments and thus their print functions ("print_getxattr", 
"print_lgetxattr")
share a same definition. The same statement applies to syscalls 
"listxattr()"
and "llistxattr()".
Function "print_syscall_ret_listxattr()" was added to print the returned 
list
of extended attributes for syscalls and was listed as a "result" function 
in file
"strace.list" for syscalls: "listxattr(), llistxattr(), flistxattr()".

Signed-off-by: Filip Bozuta 
---
 linux-user/strace.c| 126 +
 linux-user/strace.list |  21 ---
 2 files changed, 138 insertions(+), 9 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index f980451e3f..59fdb0a05f 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -830,6 +830,45 @@ print_syscall_ret_adjtimex(const struct syscallname *name, 
abi_long ret,
 }
 #endif
 
+#if defined(TARGET_NR_listxattr) || defined(TARGET_NR_llistxattr) \
+ || defined(TARGGET_NR_flistxattr)
+static void
+print_syscall_ret_listxattr(const struct syscallname *name, abi_long ret,
+abi_long arg0, abi_long arg1, abi_long arg2,
+abi_long arg3, abi_long arg4, abi_long arg5)
+{
+const char *errstr = NULL;
+
+qemu_log(" = ");
+if (ret < 0) {
+qemu_log("-1 errno=%d", errno);
+errstr = target_strerror(-ret);
+if (errstr) {
+qemu_log(" (%s)", errstr);
+}
+} else {
+qemu_log(TARGET_ABI_FMT_ld, ret);
+qemu_log(" (list = ");
+if (arg1 != 0) {
+abi_long attr = arg1;
+for (;;) {
+print_string(attr, 1);
+attr += target_strlen(attr) + 1;
+if (target_strlen(attr) == 0) {
+break;
+}
+qemu_log(",");
+}
+} else {
+qemu_log("NULL");
+}
+qemu_log(")");
+}
+
+qemu_log("\n");
+}
+#endif
+
 UNUSED static struct flags access_flags[] = {
 FLAG_GENERIC(F_OK),
 FLAG_GENERIC(R_OK),
@@ -1637,6 +1676,93 @@ print_fcntl(const struct syscallname *name,
 #define print_fcntl64   print_fcntl
 #endif
 
+#ifdef TARGET_NR_fgetxattr
+static void
+print_fgetxattr(const struct syscallname *name,
+abi_long arg0, abi_long arg1, abi_long arg2,
+abi_long arg3, abi_long arg4, abi_long arg5)
+{
+print_syscall_prologue(name);
+print_raw_param("%d", arg0, 0);
+print_string(arg1, 0);
+print_pointer(arg2, 0);
+print_raw_param(TARGET_FMT_lu, arg3, 1);
+print_syscall_epilogue(name);
+}
+#endif
+
+#ifdef TARGET_NR_flistxattr
+static void
+print_flistxattr(const struct syscallname *name,
+abi_long arg0, abi_long arg1, abi_long arg2,
+abi_long arg3, abi_long arg4, abi_long arg5)
+{
+print_syscall_prologue(name);
+print_raw_param("%d", arg0, 0);
+print_pointer(arg1, 0);
+print_raw_param(TARGET_FMT_lu, arg2, 1);
+print_syscall_epilogue(name);
+}
+#endif
+
+#if defined(TARGET_NR_getxattr) || defined(TARGET_NR_lgetxattr)
+static void
+print_getxattr(const struct syscallname *name,
+abi_long arg0, abi_long arg1, abi_long arg2,
+abi_long arg3, abi_long arg4, abi_long arg5)
+{
+print_sys

[PATCH v8 09/10] Makefile: Allow target-specific optional Kconfig

[Philippe Mathieu-Daudé]

Allow use of target-specific Kconfig file.

Reviewed-by: Richard Henderson 
Reviewed-by: Alex Bennée 
Signed-off-by: Philippe Mathieu-Daudé 
---
 Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index aef466b0b9..6bc4fed7bd 100644
--- a/Makefile
+++ b/Makefile
@@ -423,11 +423,13 @@ MINIKCONF_INPUTS = $(SRC_PATH)/Kconfig.host \
$(SRC_PATH)/accel/Kconfig \
$(SRC_PATH)/hw/Kconfig
 MINIKCONF_DEPS = $(MINIKCONF_INPUTS) \
- $(wildcard $(SRC_PATH)/hw/*/Kconfig)
+ $(wildcard $(SRC_PATH)/hw/*/Kconfig) \
+ $(wildcard $(SRC_PATH)/target/*/Kconfig)
 MINIKCONF = $(PYTHON) $(SRC_PATH)/scripts/minikconf.py
 
 $(SUBDIR_DEVICES_MAK): %/config-devices.mak: default-configs/%.mak 
$(MINIKCONF_DEPS) $(BUILD_DIR)/config-host.mak
$(call quiet-command, $(MINIKCONF) $(MINIKCONF_ARGS) \
+   $(wildcard $(SRC_PATH)/target/$(call base-arch, $@)/Kconfig) \
> $@.tmp, "GEN", "$@.tmp")
$(call quiet-command, if test -f $@; then \
  if cmp -s $@.old $@; then \
-- 
2.21.3

[PATCH v2 2/6] linux-user: Add strace support for a group of syscalls

[Filip Bozuta]

From: Filip Bozuta 

This patch implements strace argument printing functionality for following 
syscalls:

*acct - switch process accounting on or off

int acct(const char *filename)
man page: https://www.man7.org/linux/man-pages/man2/acct.2.html

*fsync, fdatasync - synchronize a file's in-core state with storage device

int fsync(int fd)
int fdatasync(int fd)
man page: https://www.man7.org/linux/man-pages/man2/fsync.2.html

*listen - listen for connections on a socket

int listen(int sockfd, int backlog)
man page: https://www.man7.org/linux/man-pages/man2/listen.2.html

Implementation notes:

Syscall acct() takes string as its only argument and thus a separate
print function "print_acct" is stated in file "strace.list". This
function is defined and implemented in "strace.c" by using an
existing function used to print string arguments: "print_string()".
All the other syscalls have only primitive argument types, so the
rest of the implementation was handled by stating an appropriate
printing format in file "strace.list".

Signed-off-by: Filip Bozuta 
Reviewed-by: Laurent Vivier 
---
 linux-user/strace.c| 13 -
 linux-user/strace.list |  8 
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 23f7c386b5..f980451e3f 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1361,6 +1361,18 @@ print_access(const struct syscallname *name,
 }
 #endif
 
+#ifdef TARGET_NR_acct
+static void
+print_acct(const struct syscallname *name,
+abi_long arg0, abi_long arg1, abi_long arg2,
+abi_long arg3, abi_long arg4, abi_long arg5)
+{
+print_syscall_prologue(name);
+print_string(arg0, 1);
+print_syscall_epilogue(name);
+}
+#endif
+
 #ifdef TARGET_NR_brk
 static void
 print_brk(const struct syscallname *name,
@@ -1625,7 +1637,6 @@ print_fcntl(const struct syscallname *name,
 #define print_fcntl64   print_fcntl
 #endif
 
-
 #ifdef TARGET_NR_futimesat
 static void
 print_futimesat(const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index d49a1e92a8..fb9799e7e6 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -13,7 +13,7 @@
 { TARGET_NR_access, "access" , NULL, print_access, NULL },
 #endif
 #ifdef TARGET_NR_acct
-{ TARGET_NR_acct, "acct" , NULL, NULL, NULL },
+{ TARGET_NR_acct, "acct" , NULL, print_acct, NULL },
 #endif
 #ifdef TARGET_NR_add_key
 { TARGET_NR_add_key, "add_key" , NULL, NULL, NULL },
@@ -215,7 +215,7 @@
 { TARGET_NR_fcntl64, "fcntl64" , NULL, print_fcntl64, NULL },
 #endif
 #ifdef TARGET_NR_fdatasync
-{ TARGET_NR_fdatasync, "fdatasync" , NULL, NULL, NULL },
+{ TARGET_NR_fdatasync, "fdatasync" , "%s(%d)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_fgetxattr
 { TARGET_NR_fgetxattr, "fgetxattr" , NULL, NULL, NULL },
@@ -251,7 +251,7 @@
 { TARGET_NR_fstatfs64, "fstatfs64" , "%s(%d,%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_fsync
-{ TARGET_NR_fsync, "fsync" , NULL, NULL, NULL },
+{ TARGET_NR_fsync, "fsync" , "%s(%d)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_ftime
 { TARGET_NR_ftime, "ftime" , NULL, NULL, NULL },
@@ -492,7 +492,7 @@
 { TARGET_NR_Linux, "Linux" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_listen
-{ TARGET_NR_listen, "listen" , NULL, NULL, NULL },
+{ TARGET_NR_listen, "listen" , "%s(%d,%d)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_listxattr
 { TARGET_NR_listxattr, "listxattr" , NULL, NULL, NULL },
-- 
2.17.1

Re: [PATCH v8 05/10] Makefile: Remove dangerous EOL trailing backslash

[Philippe Mathieu-Daudé]

On 6/8/20 6:38 PM, Philippe Mathieu-Daudé wrote:
> One might get caught trying to understand unexpected Makefile
> behavior. Trailing backslash can help to split very long lines,
> but are rather dangerous when nothing follow. Preserve other
> developers debugging time by removing this one.
> 
> Reviewed-by: Thomas Huth 
> Reviewed-by: Alistair Francis 

I forgot here:
Reviewed-by: Richard Henderson 

> Reviewed-by: Alex Bennée 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index d1af126ea1..52956b16fd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -420,7 +420,7 @@ MINIKCONF_ARGS = \
>  
>  MINIKCONF_INPUTS = $(SRC_PATH)/Kconfig.host $(SRC_PATH)/hw/Kconfig
>  MINIKCONF_DEPS = $(MINIKCONF_INPUTS) $(wildcard $(SRC_PATH)/hw/*/Kconfig)
> -MINIKCONF = $(PYTHON) $(SRC_PATH)/scripts/minikconf.py \
> +MINIKCONF = $(PYTHON) $(SRC_PATH)/scripts/minikconf.py
>  
>  $(SUBDIR_DEVICES_MAK): %/config-devices.mak: default-configs/%.mak 
> $(MINIKCONF_DEPS) $(BUILD_DIR)/config-host.mak
>   $(call quiet-command, $(MINIKCONF) $(MINIKCONF_ARGS) > $@.tmp, "GEN", 
> "$@.tmp")
> 

[PATCH v8 05/10] Makefile: Remove dangerous EOL trailing backslash

[Philippe Mathieu-Daudé]

One might get caught trying to understand unexpected Makefile
behavior. Trailing backslash can help to split very long lines,
but are rather dangerous when nothing follow. Preserve other
developers debugging time by removing this one.

Reviewed-by: Thomas Huth 
Reviewed-by: Alistair Francis 
Reviewed-by: Alex Bennée 
Signed-off-by: Philippe Mathieu-Daudé 
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index d1af126ea1..52956b16fd 100644
--- a/Makefile
+++ b/Makefile
@@ -420,7 +420,7 @@ MINIKCONF_ARGS = \
 
 MINIKCONF_INPUTS = $(SRC_PATH)/Kconfig.host $(SRC_PATH)/hw/Kconfig
 MINIKCONF_DEPS = $(MINIKCONF_INPUTS) $(wildcard $(SRC_PATH)/hw/*/Kconfig)
-MINIKCONF = $(PYTHON) $(SRC_PATH)/scripts/minikconf.py \
+MINIKCONF = $(PYTHON) $(SRC_PATH)/scripts/minikconf.py
 
 $(SUBDIR_DEVICES_MAK): %/config-devices.mak: default-configs/%.mak 
$(MINIKCONF_DEPS) $(BUILD_DIR)/config-host.mak
$(call quiet-command, $(MINIKCONF) $(MINIKCONF_ARGS) > $@.tmp, "GEN", 
"$@.tmp")
-- 
2.21.3