date:20200116

Kevin Wolf  writes:

> Am 16.01.2020 um 14:00 hat Markus Armbruster geschrieben:
>> Kevin Wolf  writes:
>> > I have no idea if we will eventually get a case where the command wants
>> > to behave different between the two modes and actually has use for a
>> > coroutine. I hope not.
>> >
>> > But using two bools rather than a single enum keeps the code simple and
>> > leaves us all options open if it turns out that we do have a use case.
>> 
>> I can buy the argument "the two are conceptually orthogonal, although we
>> don't haven't found a use for one of the four cases".
>> 
>> Let's review the four combinations of the two flags once more:
>> 
>> * allow-oob: false, coroutine: false
>> 
>>   Handler runs in main loop, outside coroutine context.  Okay.
>> 
>> * allow-oob: false, coroutine: true
>> 
>>   Handler runs in main loop, in coroutine context.  Okay.
>> 
>> * allow-oob: true, coroutine: false
>> 
>>   Handler may run in main loop or in iothread, outside coroutine
>>   context.  Okay.
>> 
>> * allow-oob: true, coroutine: true
>> 
>>   Handler may run (in main loop, in coroutine context) or (in iothread,
>>   outside coroutine context).  This "in coroutine context only with
>>   execute, not with exec-oob" behavior is a bit surprising.
>> 
>>   We could document it, noting that it may change to always run in
>>   coroutine context.  Or we simply reject this case as "not
>>   implemented".  Since we have no uses, I'm leaning towards reject.  One
>>   fewer case to test then.
>
> What would be the right mode of rejecting it?
>
> I assume we should catch it somewhere in the QAPI generator (where?) and

check_flags() in expr.py?

> then just assert in the C code that both flags aren't set at the same
> time?

I think you already do, in do_qmp_dispatch():

assert(!(oob && qemu_in_coroutine()));

Not sure that's the best spot.  Let's see when I review PATCH 3.

>> >> > @@ -194,8 +195,9 @@ out:
>> >> >  return ret
>> >> >  
>> >> >  
>> >> > -def gen_register_command(name, success_response, allow_oob, 
>> >> > allow_preconfig):
>> >> > -options = []
>> >> > +def gen_register_command(name: str, success_response: bool, allow_oob: 
>> >> > bool,
>> >> > + allow_preconfig: bool, coroutine: bool) -> 
>> >> > str:
>> >> > +options = [] # type: List[str]
>> 
>> One more: this is a PEP 484 type hint.  With Python 3, we can use PEP
>> 526 instead:
>> 
>>   options: List[str] = []
>> 
>> I think we should.
>
> This requires Python 3.6, unfortunately. The minimum requirement for
> building QEMU is 3.5.

*Sigh*

>> >> Some extra churn due to type hints here.  Distracting.  Suggest not to
>> >> mix adding type hints to existing code with feature work.
>> >
>> > If you would be open for a compromise, I could leave options
>> > unannotated, but keep the typed parameter list.
>> 
>> Keeping just the function annotation is much less distracting.  I can't
>> reject that with a "separate patches for separate things" argument.
>> 
>> I'd still prefer not to, because:
>> 
>> * If we do add systematic type hints in the near future, then delaying
>>   this one until then shouldn't hurt your productivity.
>> 
>> * If we don't, this lone one won't help your productivity much, but
>>   it'll look out of place.
>> 
>> I really don't want us to add type hints as we go, because such
>> open-ended "while we touch it anyway" conversions take forever and a
>> day.  Maximizes the confusion integral over time.
>
> I think it's a first time that I'm asked not to document things, but
> I'll remove them.

I'm asking you not to mix documenting existing code with adding a
new feature to it in the same patch.

Hopefully, that won't lead to the documentation getting dropped for
good.  That would be sad.

[PATCH 2/2] virtio-scsi: convert to new virtio_delete_queue

From: Pan Nengyuan 

Use virtio_delete_queue to make it more clear.

Signed-off-by: Pan Nengyuan 
---
 hw/scsi/virtio-scsi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 858b3aaccb..d3af42ef92 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -945,10 +945,10 @@ void virtio_scsi_common_unrealize(DeviceState *dev)
 VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
 int i;
 
-virtio_del_queue(vdev, 0);
-virtio_del_queue(vdev, 1);
+virtio_delete_queue(vs->ctrl_vq);
+virtio_delete_queue(vs->event_vq);
 for (i = 0; i < vs->conf.num_queues; i++) {
-virtio_del_queue(vdev, i + 2);
+virtio_delete_queue(vs->cmd_vqs[i]);
 }
 g_free(vs->cmd_vqs);
 virtio_cleanup(vdev);
-- 
2.21.0.windows.1

[PATCH 1/2] virtio-scsi: delete vqs in unrealize to avoid memleaks

From: Pan Nengyuan 

This patch fix memleaks when attaching/detaching virtio-scsi device, the
memory leak stack is as follow:

Direct leak of 21504 byte(s) in 3 object(s) allocated from:
  #0 0x7f491f2f2970 (/lib64/libasan.so.5+0xef970)  ??:?
  #1 0x7f491e94649d (/lib64/libglib-2.0.so.0+0x5249d)  ??:?
  #2 0x564d0f3919fa (./x86_64-softmmu/qemu-system-x86_64+0x2c3e9fa)  
/mnt/sdb/qemu/hw/virtio/virtio.c:2333
  #3 0x564d0f2eca55 (./x86_64-softmmu/qemu-system-x86_64+0x2b99a55)  
/mnt/sdb/qemu/hw/scsi/virtio-scsi.c:912
  #4 0x564d0f2ece7b (./x86_64-softmmu/qemu-system-x86_64+0x2b99e7b)  
/mnt/sdb/qemu/hw/scsi/virtio-scsi.c:924
  #5 0x564d0f39ee47 (./x86_64-softmmu/qemu-system-x86_64+0x2c4be47)  
/mnt/sdb/qemu/hw/virtio/virtio.c:3531
  #6 0x564d0f980224 (./x86_64-softmmu/qemu-system-x86_64+0x322d224)  
/mnt/sdb/qemu/hw/core/qdev.c:865

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 hw/scsi/virtio-scsi.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 4bc73a370e..858b3aaccb 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -943,7 +943,13 @@ void virtio_scsi_common_unrealize(DeviceState *dev)
 {
 VirtIODevice *vdev = VIRTIO_DEVICE(dev);
 VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
+int i;
 
+virtio_del_queue(vdev, 0);
+virtio_del_queue(vdev, 1);
+for (i = 0; i < vs->conf.num_queues; i++) {
+virtio_del_queue(vdev, i + 2);
+}
 g_free(vs->cmd_vqs);
 virtio_cleanup(vdev);
 }
-- 
2.21.0.windows.1

[PATCH 0/2] delete virtio queues in virtio_scsi_unrealize

From: Pan Nengyuan 

This serie patch fix memleaks when detaching virtio-scsi device. 
1. use old virtio_del_queue to fix memleaks, it's easier for stable branches to 
merge.
   As the discussion in 
https://lists.nongnu.org/archive/html/qemu-devel/2020-01/msg02903.html

2. replace virtio_del_queue to virtio_delete_queue to make it more clear.

Pan Nengyuan (2):
  virtio-scsi: delete vqs in unrealize to avoid memleaks
  virtio-scsi: convert to new virtio_delete_queue

 hw/scsi/virtio-scsi.c | 6 ++
 1 file changed, 6 insertions(+)

-- 
2.21.0.windows.1

Re: [PATCH v3 1/4] qapi: Add a 'coroutine' flag for commands

Markus Armbruster  writes:

> Kevin Wolf  writes:
>
>> Am 15.01.2020 um 15:59 hat Markus Armbruster geschrieben:
>>> Kevin Wolf  writes:
>>> 
>>> > This patch adds a new 'coroutine' flag to QMP command definitions that
>>> > tells the QMP dispatcher that the command handler is safe to be run in a
>>> > coroutine.
>>> >
>>> > Signed-off-by: Kevin Wolf 
>>> > Reviewed-by: Marc-André Lureau 
>>> > ---
>>> >  tests/qapi-schema/qapi-schema-test.json |  1 +
>>> >  docs/devel/qapi-code-gen.txt|  4 
>>> >  include/qapi/qmp/dispatch.h |  1 +
>>> >  tests/test-qmp-cmds.c   |  4 
>>> >  scripts/qapi/commands.py| 17 +++--
>>> >  scripts/qapi/doc.py |  2 +-
>>> >  scripts/qapi/expr.py|  4 ++--
>>> >  scripts/qapi/introspect.py  |  2 +-
>>> >  scripts/qapi/schema.py  |  9 ++---
>>> >  tests/qapi-schema/qapi-schema-test.out  |  2 ++
>>> >  tests/qapi-schema/test-qapi.py  |  7 ---
>>> >  11 files changed, 37 insertions(+), 16 deletions(-)
>>> >
>>> > diff --git a/tests/qapi-schema/qapi-schema-test.json 
>>> > b/tests/qapi-schema/qapi-schema-test.json
>>> > index 9abf175fe0..1a850fe171 100644
>>> > --- a/tests/qapi-schema/qapi-schema-test.json
>>> > +++ b/tests/qapi-schema/qapi-schema-test.json
>>> > @@ -147,6 +147,7 @@
>>> >'returns': 'UserDefTwo' }
>>> >  
>>> >  { 'command': 'cmd-success-response', 'data': {}, 'success-response': 
>>> > false }
>>> > +{ 'command': 'coroutine-cmd', 'data': {}, 'coroutine': true }
>>> >  
>>> >  # Returning a non-dictionary requires a name from the whitelist
>>> >  { 'command': 'guest-get-time', 'data': {'a': 'int', '*b': 'int' },
>>> > diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
>>> > index 45c93a43cc..753f6711d3 100644
>>> > --- a/docs/devel/qapi-code-gen.txt
>>> > +++ b/docs/devel/qapi-code-gen.txt
>>> > @@ -457,6 +457,7 @@ Syntax:
>>> >  '*gen': false,
>>> >  '*allow-oob': true,
>>> >  '*allow-preconfig': true,
>>> > +'*coroutine': true,
>>> >  '*if': COND,
>>> >  '*features': FEATURES }
>>> >  
>>> > @@ -581,6 +582,9 @@ before the machine is built.  It defaults to false.  
>>> > For example:
>>> >  QMP is available before the machine is built only when QEMU was
>>> >  started with --preconfig.
>>> >  
>>> > +Member 'coroutine' tells the QMP dispatcher whether the command handler
>>> > +is safe to be run in a coroutine. It defaults to false.
>>> 
>>> Two spaces after sentence-ending period, for consistency with the rest
>>> of the file.
>>
>> Ok.
>>
>>> As discussed in review of prior versions, coroutine-safety is an
>>> implementation detail that should not be exposed to management
>>> applications.  Therefore, we want a flag, not a feature.
>>> 
>>> As far as I can tell, the new flag has no effect until PATCH 3 puts it
>>> to use.  That's okay.
>>> 
>>> The doc update tells us when we may say 'coroutine': true, namely when
>>> the handler function is coroutine-safe.  It doesn't quite tell us what
>>> difference it makes, or rather will make after PATCH 3.  I think it
>>> should.
>>
>> Fair requirement. Can I describe it as if patch 3 were already in? That
>> is, the documentation says that the handler _will_ be run in a coroutine
>> rather than _may_ run it in a coroutine?
>
> Your choice.  If you choose to pretend PATCH 3 was in, have your commit
> message point that out.
>
>>> In review of a prior version, Marc-André wondered whether keeping
>>> allow-oob and coroutine separate makes sense.  Recall qapi-code-gen.txt:
>>> 
>>> An OOB-capable command handler must satisfy the following conditions:
>>> 
>>> - It terminates quickly.
>>> - It does not invoke system calls that may block.
>>> - It does not access guest RAM that may block when userfaultfd is
>>>   enabled for postcopy live migration.
>>> - It takes only "fast" locks, i.e. all critical sections protected by
>>>   any lock it takes also satisfy the conditions for OOB command
>>>   handler code.
>>> 
>>> The restrictions on locking limit access to shared state.  Such access
>>> requires synchronization, but OOB commands can't take the BQL or any
>>> other "slow" lock.
>>> 
>>> Kevin, does this rule out coroutine use?
>>
>> Not strictly, though I also can't think of a case where you would want
>> to use a coroutine with these requirements.
>>
>> If I understand correctly, OOB-capable commands can be run either OOB
>> with 'exec-oob' or like normal commands with 'execute'.
>
> Correct.
>
>> If an OOB
>> handler is marked as coroutine-safe, 'execute' will run it in a
>> coroutine (and the restriction above don't apply) and 'exec-oob' will
>> run it outside of coroutine context.
>
> Let me convince myself you're right.
>
> Cases before this series:

Re: [PATCH v1] vnc: fix VNC artifacts

2020-01-16 Thread Cameron Esfahani via

Yes.  Personally, I'd also take the change to vnc-enc-zrle.c: because 
vs->zrle->zlib is reset at the top of the function, using vs->zrle->zlib.offset 
in determining zstream->next_out and zstream->avail_out is useless.

Cameron Esfahani
di...@apple.com

"All that is necessary for the triumph of evil is that good men do nothing."

Edmund Burke



> On Jan 16, 2020, at 11:45 PM, Gerd Hoffmann  wrote:
> 
> On Thu, Jan 16, 2020 at 07:50:58PM -0800, Cameron Esfahani wrote:
>> Remove VNC optimization to reencode framebuffer update as raw if it's
>> smaller than the default encoding.  QEMU's implementation was naive and
>> didn't account for the ZLIB z_stream mutating with each compression.  Just
>> saving and restoring the output buffer offset wasn't sufficient to "rewind"
>> the previous encoding.  Considering that ZRLE is never larger than raw and
>> even though ZLIB can occasionally be fractionally larger than raw, the
>> overhead of implementing this optimization correctly isn't worth it.
> 
> So just revert de3f7de7f4e257 then ...
> 
>> In my investigation, ZRLE always compresses better than ZLIB so
>> prioritize ZRLE over ZLIB, even if the client hints that ZLIB is
>> preferred.
> 
> ... and make this a separate patch?
> 
> cheers,
>  Gerd
>

Re: [PATCH v1] vnc: fix VNC artifacts

2020-01-16 Thread Gerd Hoffmann

On Thu, Jan 16, 2020 at 07:50:58PM -0800, Cameron Esfahani wrote:
> Remove VNC optimization to reencode framebuffer update as raw if it's
> smaller than the default encoding.  QEMU's implementation was naive and
> didn't account for the ZLIB z_stream mutating with each compression.  Just
> saving and restoring the output buffer offset wasn't sufficient to "rewind"
> the previous encoding.  Considering that ZRLE is never larger than raw and
> even though ZLIB can occasionally be fractionally larger than raw, the
> overhead of implementing this optimization correctly isn't worth it.

So just revert de3f7de7f4e257 then ...

> In my investigation, ZRLE always compresses better than ZLIB so
> prioritize ZRLE over ZLIB, even if the client hints that ZLIB is
> preferred.

... and make this a separate patch?

cheers,
  Gerd

[Bug 1860056] Re: mips binaries segfault

Does the problem exist using c hello world and gcc?

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

Title:
  mips binaries segfault

Status in QEMU:
  New

Bug description:
  Hello World appears to segfault with qemu mips, on a Debian 10.0.0
  Buster amd64 host.

  Example:

  
  $ cat mips/test/hello.cpp 
  #include 
  using std::cout;

  int main() {
  cout << "Hello World!\n";
  return 0;
  }

  $ mips-linux-gnu-g++ -o hello hello.cpp && ./hello
  qemu: uncaught target signal 11 (Segmentation fault) - core dumped

  Note that 64-bit MIPS and little endian 32-bit MIPS qemu work fine.
  The problem is limited to big endian 32-bit MIPS.

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

Re: [PATCH v2] uas: fix super speed bMaxPacketSize0


On 1/17/20 8:37 AM, Gerd Hoffmann wrote:

For usb2 bMaxPacketSize0 is "n", for usb3 it is "1 << n",
so it must be 9 not 64 ...

rom "Universal Serial Bus 3.1 Specification":


Oops typo "From" ;) You can obviously fix that directly on your tree, no 
need for v3.


Thanks for the v2!



If the device is operating at Gen X speed, the bMaxPacketSize0
field shall be set to 09H indicating a 512-byte maximum packet.
An Enhanced SuperSpeed device shall not support any other maximum
packet sizes for the default control pipe (endpoint 0) control
endpoint.

We now announce a 512-byte maximum packet.

Fixes: 89a453d4a5c ("uas-uas: usb3 streams")
Reported-by: Benjamin David Lunt 
Signed-off-by: Gerd Hoffmann 
Reviewed-by: Philippe Mathieu-Daudé 
---
  hw/usb/dev-uas.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index 6d6d1073b907..1bc4dd4fafb8 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -303,7 +303,7 @@ static const USBDescDevice desc_device_high = {
  
  static const USBDescDevice desc_device_super = {

  .bcdUSB= 0x0300,
-.bMaxPacketSize0   = 64,
+.bMaxPacketSize0   = 9,
  .bNumConfigurations= 1,
  .confs = (USBDescConfig[]) {
  {

Re: [PATCH v22 5/9] ACPI: Record the Generic Error Status Block address


On 1/8/20 12:32 PM, Dongjiu Geng wrote:

Record the GHEB address via fw_cfg file, when recording
a error to CPER, it will use this address to find out
Generic Error Data Entries and write the error.

Make the HEST GHES to a GED device.

Signed-off-by: Dongjiu Geng 
Signed-off-by: Xiang Zheng 
---
  hw/acpi/generic_event_device.c | 15 ++-
  hw/acpi/ghes.c | 16 
  hw/arm/virt-acpi-build.c   | 13 -
  include/hw/acpi/generic_event_device.h |  2 ++
  include/hw/acpi/ghes.h |  6 ++
  5 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 9cee90c..9bf37e4 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -234,12 +234,25 @@ static const VMStateDescription vmstate_ged_state = {
  }
  };
  
+static const VMStateDescription vmstate_ghes_state = {

+.name = "acpi-ghes-state",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields  = (VMStateField[]) {
+VMSTATE_UINT64(ghes_addr_le, AcpiGhesState),
+VMSTATE_END_OF_LIST()
+}
+};
+
  static const VMStateDescription vmstate_acpi_ged = {
  .name = "acpi-ged",
  .version_id = 1,
  .minimum_version_id = 1,
  .fields = (VMStateField[]) {
-VMSTATE_STRUCT(ged_state, AcpiGedState, 1, vmstate_ged_state, 
GEDState),
+VMSTATE_STRUCT(ged_state, AcpiGedState, 1,
+   vmstate_ged_state, GEDState),
+VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
+   vmstate_ghes_state, AcpiGhesState),
  VMSTATE_END_OF_LIST(),
  },
  .subsections = (const VMStateDescription * []) {
diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
index 9d37798..68f4abf 100644
--- a/hw/acpi/ghes.c
+++ b/hw/acpi/ghes.c
@@ -23,6 +23,7 @@
  #include "hw/acpi/acpi.h"
  #include "hw/acpi/ghes.h"
  #include "hw/acpi/aml-build.h"
+#include "hw/acpi/generic_event_device.h"
  #include "hw/nvram/fw_cfg.h"
  #include "sysemu/sysemu.h"
  #include "qemu/error-report.h"
@@ -208,3 +209,18 @@ void acpi_build_hest(GArray *table_data, GArray 
*hardware_errors,
  build_header(linker, table_data, (void *)(table_data->data + hest_start),
  "HEST", table_data->len - hest_start, 1, NULL, "");
  }
+
+void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState *s,
+GArray *hardware_error)
+{
+size_t size = 2 * sizeof(uint64_t) + ACPI_GHES_MAX_RAW_DATA_LENGTH;
+size_t request_block_size = ACPI_GHES_ERROR_SOURCE_COUNT * size;
+
+/* Create a read-only fw_cfg file for GHES */
+fw_cfg_add_file(s, ACPI_GHES_ERRORS_FW_CFG_FILE, hardware_error->data,
+request_block_size);
+
+/* Create a read-write fw_cfg file for Address */
+fw_cfg_add_file_callback(s, ACPI_GHES_DATA_ADDR_FW_CFG_FILE, NULL, NULL,
+NULL, &(ags->ghes_addr_le), sizeof(ags->ghes_addr_le), false);
+}
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 837bbf9..c8aa94d 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -797,6 +797,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables 
*tables)
  unsigned dsdt, xsdt;
  GArray *tables_blob = tables->table_data;
  MachineState *ms = MACHINE(vms);
+AcpiGedState *acpi_ged_state;
  
  table_offsets = g_array_new(false, true /* clear */,

  sizeof(uint32_t));
@@ -831,7 +832,9 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables 
*tables)
  acpi_add_table(table_offsets, tables_blob);
  build_spcr(tables_blob, tables->linker, vms);
  
-if (vms->ras) {

+acpi_ged_state = ACPI_GED(object_resolve_path_type("", TYPE_ACPI_GED,
+   NULL));


Testing vms->ras first is cheaper than calling 
object_resolve_path_type(). Since some people are spending lot of time 
to reduce VM boot time, it might be worth considering.



+if (acpi_ged_state &&  vms->ras) {
  acpi_add_table(table_offsets, tables_blob);
  build_ghes_error_table(tables->hardware_errors, tables->linker);
  acpi_build_hest(tables_blob, tables->hardware_errors,
@@ -925,6 +928,7 @@ void virt_acpi_setup(VirtMachineState *vms)
  {
  AcpiBuildTables tables;
  AcpiBuildState *build_state;
+AcpiGedState *acpi_ged_state;
  
  if (!vms->fw_cfg) {

  trace_virt_acpi_setup();
@@ -955,6 +959,13 @@ void virt_acpi_setup(VirtMachineState *vms)
  fw_cfg_add_file(vms->fw_cfg, ACPI_BUILD_TPMLOG_FILE, tables.tcpalog->data,
  acpi_data_len(tables.tcpalog));
  
+acpi_ged_state = ACPI_GED(object_resolve_path_type("", TYPE_ACPI_GED,

+   NULL));
+if (acpi_ged_state && vms->ras) {
+acpi_ghes_add_fw_cfg(&acpi_ged_state->ghes_state,
+ vms->fw_c

[PATCH v2] uas: fix super speed bMaxPacketSize0

2020-01-16 Thread Gerd Hoffmann

For usb2 bMaxPacketSize0 is "n", for usb3 it is "1 << n",
so it must be 9 not 64 ...

rom "Universal Serial Bus 3.1 Specification":

   If the device is operating at Gen X speed, the bMaxPacketSize0
   field shall be set to 09H indicating a 512-byte maximum packet.
   An Enhanced SuperSpeed device shall not support any other maximum
   packet sizes for the default control pipe (endpoint 0) control
   endpoint.

We now announce a 512-byte maximum packet.

Fixes: 89a453d4a5c ("uas-uas: usb3 streams")
Reported-by: Benjamin David Lunt 
Signed-off-by: Gerd Hoffmann 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/usb/dev-uas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index 6d6d1073b907..1bc4dd4fafb8 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -303,7 +303,7 @@ static const USBDescDevice desc_device_high = {
 
 static const USBDescDevice desc_device_super = {
 .bcdUSB= 0x0300,
-.bMaxPacketSize0   = 64,
+.bMaxPacketSize0   = 9,
 .bNumConfigurations= 1,
 .confs = (USBDescConfig[]) {
 {
-- 
2.18.1

Re: [PATCH v2 85/86] numa: make exit() usage consistent

2020-01-16 Thread Thomas Huth

On 16/01/2020 18.10, Igor Mammedov wrote:
> On Thu, 16 Jan 2020 17:43:30 +0100
> Thomas Huth  wrote:
> 
>> On 15/01/2020 16.07, Igor Mammedov wrote:
>>> Signed-off-by: Igor Mammedov 
>>> ---
>>> CC: ehabk...@redhat.com
>>> ---
>>>  hw/core/numa.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/core/numa.c b/hw/core/numa.c
>>> index 3177066..47d5ea1 100644
>>> --- a/hw/core/numa.c
>>> +++ b/hw/core/numa.c
>>> @@ -718,7 +718,7 @@ void numa_complete_configuration(MachineState *ms)
>>>  /* Report large node IDs first, to make mistakes easier to spot */
>>>  if (!numa_info[i].present) {
>>>  error_report("numa: Node ID missing: %d", i);
>>> -exit(1);
>>> +exit(EXIT_FAILURE);
>>>  }
>>>  }
>>>  
>>> @@ -759,7 +759,7 @@ void numa_complete_configuration(MachineState *ms)
>>>  error_report("total memory for NUMA nodes (0x%" PRIx64 ")"
>>>   " should equal RAM size (0x" RAM_ADDR_FMT ")",
>>>   numa_total, ram_size);
>>> -exit(1);
>>> +exit(EXIT_FAILURE);
>>>  }
>>>  
>>>  if (!numa_uses_legacy_mem()) {  
>>
>> Please don't. We've had exit(1) vs. exit(EXIT_FAILURE) discussions in
>> the past already, and IIRC there was no clear conclusion which one we
>> want to use. There are examples of changes to the numeric value in our
>> git history (see d54e4d7659ebecd0e1fa7ffc3e954197e09f8a1f for example),
>> and example of the other way round (see 4d1275c24d5d64d22ec4a30ce1b6a0
>> for example).
>>
>> Your patch series here is already big enough, so I suggest to drop this
>> patch from the series. If you want to change this, please suggest an
>> update to CODING_STYLE.rst first so that we agree upon one style for
>> exit() ... otherwise somebody else might change this back into numeric
>> values in a couple of months just because they have a different taste.
> 
> Ok, will do.
> 
> There are other patches that introduce new exit(EXIT_FAILURE),
> is it fine to use that or should I stick to the style used in nearby code?

Since we don't have a consensus yet, I guess it's ok to use it ... but
adapting to the surrounding code is also a good idea, of course.

 Thomas

Re: [PATCH v22 9/9] MAINTAINERS: Add ACPI/HEST/GHES entries


Hi Peter,

On 1/16/20 5:46 PM, Peter Maydell wrote:

On Wed, 8 Jan 2020 at 11:32, Dongjiu Geng  wrote:


I and Xiang are willing to review the APEI-related patches and
volunteer as the reviewers for the HEST/GHES part.

Signed-off-by: Dongjiu Geng 
Signed-off-by: Xiang Zheng 
---
  MAINTAINERS | 9 +
  1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 387879a..5af70a5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1423,6 +1423,15 @@ F: tests/bios-tables-test.c
  F: tests/acpi-utils.[hc]
  F: tests/data/acpi/

+ACPI/HEST/GHES
+R: Dongjiu Geng 
+R: Xiang Zheng 
+L: qemu-...@nongnu.org
+S: Maintained
+F: hw/acpi/ghes.c
+F: include/hw/acpi/ghes.h
+F: docs/specs/acpi_hest_ghes.rst
+
  ppc4xx
  M: David Gibson 
  L: qemu-...@nongnu.org
--


Michael, Igor: since this new MAINTAINERS section is
moving files out of the 'ACPI/SMBIOS' section that you're
currently responsible for, do you want to provide an
acked-by: that you think this division of files makes sense?


The files are not 'moved out', Michael and Igor are still the 
maintainers of the supported ACPI/SMBIOS subsystem:


ACPI/SMBIOS
M: Michael S. Tsirkin 
M: Igor Mammedov 
S: Supported
F: include/hw/acpi/*
F: hw/acpi/*

Dongjiu and Xiang only add themselves as reviewers to get notified on 
changes on these specific files. The more eyes the better :)


The docs/specs/acpi_hest_ghes.rst document has no maintainer, as these 
others too:


- docs/specs/acpi_cpu_hotplug.txt
- docs/specs/acpi_hw_reduced_hotplug.rst
- docs/specs/acpi_mem_hotplug.txt
- docs/specs/acpi_nvdimm.txt

The only ACPI file reported as maintained in docs/specs/ is 
acpi_pci_hotplug.txt, from this entry:


PCI
M: Michael S. Tsirkin 
M: Marcel Apfelbaum 
S: Supported
F: docs/specs/*pci*

FWIW:
Reviewed-by: Philippe Mathieu-Daudé

Re: [PATCH] qapi: Fix code generation with Python 3.5

John Snow  writes:

> On 1/16/20 3:25 PM, Markus Armbruster wrote:
>> Recent commit 3e7fb5811b "qapi: Fix code generation for empty modules"
>> modules" switched QAPISchema.visit() from
>> 
>> for entity in self._entity_list:
>> 
>> effectively to
>> 
>> for mod in self._module_dict.values():
>> for entity in mod._entity_list:
>> 
>> Visits in the same order as long as .values() is in insertion order.
>> That's the case only for Python 3.6 and later.  Before, it's in some
>> arbitrary order, which results in broken generated code.
>> 
>> Fix by making self._module_dict an OrderedDict rather than a dict.
>> 
>> Fixes: 3e7fb5811baab213dcc7149c3aa69442d683c26c
>> Signed-off-by: Markus Armbruster 
>> ---
>>  scripts/qapi/schema.py | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
>> index 0bfc5256fb..5100110fa2 100644
>> --- a/scripts/qapi/schema.py
>> +++ b/scripts/qapi/schema.py
>> @@ -795,7 +795,7 @@ class QAPISchema(object):
>>  self.docs = parser.docs
>>  self._entity_list = []
>>  self._entity_dict = {}
>> -self._module_dict = {}
>> +self._module_dict = OrderedDict()
>>  self._schema_dir = os.path.dirname(fname)
>>  self._make_module(None) # built-ins
>>  self._make_module(fname)
>> 
>
> This problem has bitten me *many* times. I'm wondering if there's a
> prescription that isn't just "Wait until we can stipulate 3.6+".

No clue.

3.5 EOL is scheduled for 2020-09-13.
https://devguide.python.org/#status-of-python-branches

We support 3.5 because we support Debian 9.

We'd normally drop support for Debian 9 two years after Debian 10,
i.e. July 2021.  Assuming Debian supports it that far.  Whether they can
truly support Python 3.5 after uptstream EOL seems doubtful.

Re: [PATCH] uas: fix super speed bMaxPacketSize0


On 1/17/20 7:27 AM, Gerd Hoffmann wrote:

For usb2 bMaxPacketSize0 is "n", for usb3 it is "1 << n",
so it must be 9 not 64 ...


Maybe add in description:

From "Universal Serial Bus 3.1 Specification":

  If the device is operating at Gen X speed, the bMaxPacketSize0
  field shall be set to 09H indicating a 512-byte maximum packet.
  An Enhanced SuperSpeed device shall not support any other maximum
  packet sizes for the default control pipe (endpoint 0) control
  endpoint.

We now announce a 512-byte maximum packet.

Fixes: 89a453d4a5c


Reported-by: f...@fysnet.net


https://wiki.qemu.org/Contribute/SubmitAPatch#Patch_emails_must_include_a_Signed-off-by:_line

"Please use your real name to sign a patch (not an alias or acronym)."

OK this is not about the author name but reporter name.

Gerd, I think you we use 'Reported-by: Benjamin David Lunt 
' instead, which was previously used in commit 
9da82227caa7 (except if Ben refuses obviously).


Reviewed-by: Philippe Mathieu-Daudé 


Signed-off-by: Gerd Hoffmann 
---
  hw/usb/dev-uas.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index 6d6d1073b907..1bc4dd4fafb8 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -303,7 +303,7 @@ static const USBDescDevice desc_device_high = {
  
  static const USBDescDevice desc_device_super = {

  .bcdUSB= 0x0300,
-.bMaxPacketSize0   = 64,
+.bMaxPacketSize0   = 9,
  .bNumConfigurations= 1,
  .confs = (USBDescConfig[]) {
  {

[Bug 1860053] Re: Possible lack of precision when calling clock_gettime via vDSO on user mode ppc64le

** Tags added: linux-user ppc

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

Title:
  Possible lack of precision when calling clock_gettime via vDSO on user
  mode ppc64le

Status in QEMU:
  New

Bug description:
  Occurs on QEMU v4.2.0 run on docker (via the qemu-user-static:v4.2.0-2
  image) on an AMD64 Ubuntu 18.04.3 LTS machine provided by travis-
  ci.org.

  From golang's https://github.com/golang/go/issues/36592:

  It was discovered that golang's time.NewTicker() and time.Sleep()
  malfunction when a compiled application was run via QEMU's ppc64le
  emulator in user mode.

  The methods did not malfunction on actual PowerPC hardware or when the
  same golang application was compiled for golang's arm, arm64 or 386
  targets and was run via user mode QEMU on the same system.

  Curiously, the methods also worked when the program was compiled under
  go 1.11, but do malfunction in go 1.12 and 1.13.

  It was identified the change in behaviour was most likely attributable to 
golang switching to using vSDO for calling clock_gettime() on PowerPC 64 
architectures in 1.12. I.E:
  https://github.com/golang/go/commit/dbd8af74723d2c98cbdcc70f7e2801f69b57ac5b

  We therefore suspect there may be a bug in QEMU's user-mode emulation
  of ppc64le as relates to vDSO calls to clock_gettime().

  The nature of the malfunction of time.NewTicker() and time.Sleep() is
  such that sleeps or ticks with a granularity of less than one second
  do not appear to be possible (they all revert to 1 second
  sleeps/ticks). Could it be that the nanoseconds field of
  clock_gettime() is getting lost in the vDSO version but not in the
  syscall? Or some other issue calling these methods via vDSO?

  Thanks in advance.

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

[Bug 1859418] Re: disk driver with iothread setting hangs live migrations

2020-01-16 Thread Mark Zealey

I will try the newest version as you suggest. However please note that
this is a redhat/centos 2.12 version which means it has a load of the
newest patches on it so probably closer to a 4-series than real 2.12...

Mark

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

Title:
  disk driver with iothread setting hangs live migrations

Status in QEMU:
  Incomplete

Bug description:
  Per report raised at
  https://bugzilla.redhat.com/show_bug.cgi?id=1790093

  Description of problem:

  A disk driver definition using iothread parameter causes live
  migration with copy storage to hang during or just before the final
  ram sync stage.

  Interestingly, having the scsi controller as a separate iothread does
  not trigger the issue.

  Version-Release number of selected component (if applicable):

  I can reproduce this on centos7 with qemu-ev and with centos 8:

  qemu-kvm-ev-2.12.0-33.1.el7_7.4.x86_64
  qemu-kvm-2.12.0-65.module_el8.0.0+189+f9babebb.5.x86_64

  Steps to Reproduce:
  1. Create a definition with 1 iothread on the disk image:



  2. Issue a live migrate request like: virsh migrate --live --copy-storage-all 
vm qemu+tcp://remote/system
  3. Live migrate on source copies storage and then hangs at 80-99%, I guess 
during the ram copy phase.

  Keeping exactly the same config but without the iothread on the disk
  driver has successful migrations every time.

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

Re: [PATCH 2/4] tests/tcg/aarch64: Fix compilation parameters for pauth-%


On 1/17/20 12:08 AM, Richard Henderson wrote:

Hiding the required architecture within asm() is not correct.
Add it to the cflags of the (cross-) compiler.

Signed-off-by: Richard Henderson 
---
  tests/tcg/aarch64/pauth-1.c   | 2 --
  tests/tcg/aarch64/pauth-2.c   | 2 --
  tests/tcg/aarch64/Makefile.target | 1 +
  3 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/tests/tcg/aarch64/pauth-1.c b/tests/tcg/aarch64/pauth-1.c
index a3c1443cd0..ea0984ea82 100644
--- a/tests/tcg/aarch64/pauth-1.c
+++ b/tests/tcg/aarch64/pauth-1.c
@@ -2,8 +2,6 @@
  #include 
  #include 
  
-asm(".arch armv8.4-a");

-
  #ifndef PR_PAC_RESET_KEYS
  #define PR_PAC_RESET_KEYS  54
  #define PR_PAC_APDAKEY (1 << 2)
diff --git a/tests/tcg/aarch64/pauth-2.c b/tests/tcg/aarch64/pauth-2.c
index 2fe030ba3d..9bba0beb63 100644
--- a/tests/tcg/aarch64/pauth-2.c
+++ b/tests/tcg/aarch64/pauth-2.c
@@ -1,8 +1,6 @@
  #include 
  #include 
  
-asm(".arch armv8.4-a");

-
  void do_test(uint64_t value)
  {
  uint64_t salt1, salt2;
diff --git a/tests/tcg/aarch64/Makefile.target 
b/tests/tcg/aarch64/Makefile.target
index df3fe8032c..374c8d6830 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -20,6 +20,7 @@ run-fcvt: fcvt
  # Pauth Tests
  AARCH64_TESTS += pauth-1 pauth-2
  run-pauth-%: QEMU_OPTS += -cpu max
+pauth-%: CFLAGS += -march=armv8.3-a


Reviewed-by: Philippe Mathieu-Daudé 

  
  # Semihosting smoke test for linux-user

  AARCH64_TESTS += semihosting

[PATCH v2] hw/arm: Adjust some coding styles about memory hotplug

2020-01-16 Thread Keqian Zhu

From: zhukeqian 

There is extra indent in ACPI GED plug cb. And we can use
existing helper function to trigger hotplug handler plug.

Reviewed-by: Igor Mammedov 
Signed-off-by: Keqian Zhu 
---

v1->v2:
 - Add Igor's R-b

Cc: Shameer Kolothum 
Cc: "Michael S. Tsirkin" 
Cc: Igor Mammedov 
Cc: Peter Maydell 
---
 hw/acpi/generic_event_device.c | 2 +-
 hw/arm/virt.c  | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 9cee90cc70..55eb29d80a 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -175,7 +175,7 @@ static void acpi_ged_device_plug_cb(HotplugHandler 
*hotplug_dev,
 AcpiGedState *s = ACPI_GED(hotplug_dev);
 
 if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
-acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, dev, errp);
+acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, dev, errp);
 } else {
 error_setg(errp, "virt: device plug request for unsupported device"
" type: %s", object_get_typename(OBJECT(dev)));
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 39ab5f47e0..656b0081c2 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1934,7 +1934,6 @@ static void virt_memory_pre_plug(HotplugHandler 
*hotplug_dev, DeviceState *dev,
 static void virt_memory_plug(HotplugHandler *hotplug_dev,
  DeviceState *dev, Error **errp)
 {
-HotplugHandlerClass *hhc;
 VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
 Error *local_err = NULL;
 
@@ -1943,8 +1942,9 @@ static void virt_memory_plug(HotplugHandler *hotplug_dev,
 goto out;
 }
 
-hhc = HOTPLUG_HANDLER_GET_CLASS(vms->acpi_dev);
-hhc->plug(HOTPLUG_HANDLER(vms->acpi_dev), dev, &error_abort);
+hotplug_handler_plug(HOTPLUG_HANDLER(vms->acpi_dev),
+ dev, &error_abort);
+
 out:
 error_propagate(errp, local_err);
 }
-- 
2.19.1

Re: [PATCH v2 4/5] linux-user: Add x86_64 vsyscall page to /proc/self/maps


On 1/16/20 8:43 PM, Richard Henderson wrote:

The page isn't (necessarily) present in the host /proc/self/maps,
and even if it might be it isn't present in page_flags, and even
if it was it might not have the same set of page permissions.

The easiest thing to do, particularly when it comes to the
"[vsyscall]" note at the end of line, is to special case it.

Signed-off-by: Richard Henderson 
---
  linux-user/syscall.c | 9 +
  1 file changed, 9 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 171c0caef3..eb867a5296 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7005,6 +7005,15 @@ static int open_self_maps(void *cpu_env, int fd)
  }
  }
  
+#ifdef TARGET_X86_64

+/*
+ * We only support execution from the vsyscall page.
+ * This is as if CONFIG_LEGACY_VSYSCALL_XONLY=y from v5.3.
+ */
+dprintf(fd, "ff60-ff601000 --xp  00:00 0"
+"  [vsyscall]\n");


Can we add a definition for 0xff60ull in the previous patch, 
and use it with TARGET_PAGE_MASK here?



+#endif
+
  free(line);
  fclose(fp);

Re: [PATCH v2 5/5] linux-user: Flush out implementation of gettimeofday


On 1/16/20 8:43 PM, Richard Henderson wrote:

The first argument, timeval, is allowed to be NULL.

The second argument, timezone, was missing.  While its use is
deprecated, it is still present in the syscall.

Signed-off-by: Richard Henderson 
---
  linux-user/syscall.c | 27 +--
  1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index eb867a5296..628b4de9a1 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1219,6 +1219,23 @@ static inline abi_long 
host_to_target_timespec64(abi_ulong target_addr,
  return 0;
  }
  
+static inline abi_long copy_to_user_timezone(abi_ulong target_tz_addr,

+ struct timezone *tz)
+{
+struct target_timezone *target_tz;
+
+if (!lock_user_struct(VERIFY_WRITE, target_tz, target_tz_addr, 1)) {
+return -TARGET_EFAULT;
+}
+
+__put_user(tz->tz_minuteswest, &target_tz->tz_minuteswest);
+__put_user(tz->tz_dsttime, &target_tz->tz_dsttime);
+
+unlock_user_struct(target_tz, target_tz_addr, 1);
+
+return 0;
+}
+
  static inline abi_long copy_from_user_timezone(struct timezone *tz,
 abi_ulong target_tz_addr)
  {
@@ -8567,10 +8584,16 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
  case TARGET_NR_gettimeofday:
  {
  struct timeval tv;
-ret = get_errno(gettimeofday(&tv, NULL));
+struct timezone tz;
+
+ret = get_errno(gettimeofday(&tv, &tz));
  if (!is_error(ret)) {
-if (copy_to_user_timeval(arg1, &tv))
+if (arg1 && copy_to_user_timeval(arg1, &tv)) {
  return -TARGET_EFAULT;
+}
+if (arg2 && copy_to_user_timezone(arg2, &tz)) {
+return -TARGET_EFAULT;
+}
  }
  }
  return ret;



Reviewed-by: Philippe Mathieu-Daudé

[PATCH] uas: fix super speed bMaxPacketSize0

2020-01-16 Thread Gerd Hoffmann

For usb2 bMaxPacketSize0 is "n", for usb3 it is "1 << n",
so it must be 9 not 64 ...

Reported-by: f...@fysnet.net
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/dev-uas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index 6d6d1073b907..1bc4dd4fafb8 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -303,7 +303,7 @@ static const USBDescDevice desc_device_high = {
 
 static const USBDescDevice desc_device_super = {
 .bcdUSB= 0x0300,
-.bMaxPacketSize0   = 64,
+.bMaxPacketSize0   = 9,
 .bNumConfigurations= 1,
 .confs = (USBDescConfig[]) {
 {
-- 
2.18.1

[PATCH v2 2/2] virtio-9p-device: convert to new virtio_delete_queue

From: Pan Nengyuan 

Use virtio_delete_queue to make it more clear.

Signed-off-by: Pan Nengyuan 
---
Changes V2 to V1:
- replace virtio_del_queue to virtio_delete_queue to make it more clear.
---
 hw/9pfs/virtio-9p-device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 910dc5045e..b146387ae2 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -215,7 +215,7 @@ static void virtio_9p_device_unrealize(DeviceState *dev, 
Error **errp)
 V9fsVirtioState *v = VIRTIO_9P(dev);
 V9fsState *s = &v->state;
 
-virtio_del_queue(vdev, 0);
+virtio_delete_queue(v->vq);
 virtio_cleanup(vdev);
 v9fs_device_unrealize_common(s, errp);
 }
-- 
2.21.0.windows.1

[PATCH v2 0/2] fix memleaks in virtio_9p_device_unrealize

From: Pan Nengyuan 

v1: fix memleaks in virtio_9p_device_unrealize
v2: split patch to make it easier for stable branches to merge

Pan Nengyuan (2):
  virtio-9p-device: fix memleak in virtio_9p_device_unrealize
  virtio-9p-device: convert to new virtio_delete_queue

 hw/9pfs/virtio-9p-device.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.21.0.windows.1

[PATCH v2 1/2] virtio-9p-device: fix memleak in virtio_9p_device_unrealize

From: Pan Nengyuan 

v->vq forgot to cleanup in virtio_9p_device_unrealize, the memory leak
stack is as follow:

Direct leak of 14336 byte(s) in 2 object(s) allocated from:
  #0 0x7f819ae43970 (/lib64/libasan.so.5+0xef970)  ??:?
  #1 0x7f819872f49d (/lib64/libglib-2.0.so.0+0x5249d)  ??:?
  #2 0x55a3a58da624 (./x86_64-softmmu/qemu-system-x86_64+0x2c14624)  
/mnt/sdb/qemu/hw/virtio/virtio.c:2327
  #3 0x55a3a571bac7 (./x86_64-softmmu/qemu-system-x86_64+0x2a55ac7)  
/mnt/sdb/qemu/hw/9pfs/virtio-9p-device.c:209
  #4 0x55a3a58e7bc6 (./x86_64-softmmu/qemu-system-x86_64+0x2c21bc6)  
/mnt/sdb/qemu/hw/virtio/virtio.c:3504
  #5 0x55a3a5ebfb37 (./x86_64-softmmu/qemu-system-x86_64+0x31f9b37)  
/mnt/sdb/qemu/hw/core/qdev.c:876

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
Changes V2 to V1:
- use old function virtio_del_queue to make it easier for stable branch
to merge (suggested by Christian Schoenebeck)
---
 hw/9pfs/virtio-9p-device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index b5a7c03f26..910dc5045e 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -215,6 +215,7 @@ static void virtio_9p_device_unrealize(DeviceState *dev, 
Error **errp)
 V9fsVirtioState *v = VIRTIO_9P(dev);
 V9fsState *s = &v->state;
 
+virtio_del_queue(vdev, 0);
 virtio_cleanup(vdev);
 v9fs_device_unrealize_common(s, errp);
 }
-- 
2.21.0.windows.1

Re: [PATCH] spapr: Fail CAS if option vector table cannot be parsed

2020-01-16 Thread David Gibson

On Thu, Jan 16, 2020 at 04:34:06PM +0100, Philippe Mathieu-Daudé wrote:
> Hi Greg,
> 
> On 1/16/20 4:05 PM, Greg Kurz wrote:
> > Most of the option vector helpers have assertions to check their
> > arguments aren't null. The guest can provide an arbitrary address
> > for the CAS structure that would result in such null arguments.
> > Fail CAS with H_PARAMETER instead of aborting QEMU.
> > 
> > Signed-off-by: Greg Kurz 
> > ---
> >   hw/ppc/spapr_hcall.c |9 +
> >   1 file changed, 9 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index 84e1612595bb..051869ae20ec 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -1701,9 +1701,18 @@ static target_ulong 
> > h_client_architecture_support(PowerPCCPU *cpu,
> >   /* For the future use: here @ov_table points to the first option 
> > vector */
> >   ov_table = addr;
> > +if (!ov_table) {
> > +return H_PARAMETER;
> > +}
> 
> This doesn't look right to check ov_table, I'd check addr directly instead:
> 
> -- >8 --
> @@ -1679,12 +1679,16 @@ static target_ulong
> h_client_architecture_support(PowerPCCPU *cpu,
> 
>  cas_pvr = cas_check_pvr(spapr, cpu, &addr, &raw_mode_supported,
> &local_err);
>  if (local_err) {
>  error_report_err(local_err);
>  return H_HARDWARE;
>  }
> +if (!addr) {
> +// error_report*()
> +return H_PARAMETER;
> +}
> 
>  /* Update CPUs */
>  if (cpu->compat_pvr != cas_pvr) {
> ---
> 
> Still I'm not sure it makes sense, because the guest can also set other
> invalid addresses such addr=0x69.

Neither is correct.  As you point out this filters at most one of many
bad addresses.  And, in fact it's not even a bad address - there's no
inherent reason the CAS information couldn't be put at guest address
0.


> 
> >   ov1_guest = spapr_ovec_parse_vector(ov_table, 1);
> > +if (!ov1_guest) {
> > +return H_PARAMETER;
> > +}
> 
> This one is OK (unlikely case where vector 1 isn't present).
> 
> >   ov5_guest = spapr_ovec_parse_vector(ov_table, 5);
> > +if (!ov5_guest) {
> > +return H_PARAMETER;
> > +}
> 
> This one is OK too (unlikely case where vector 5 isn't present).
> 
> >   if (spapr_ovec_test(ov5_guest, OV5_MMU_BOTH)) {
> >   error_report("guest requested hash and radix MMU, which is 
> > invalid.");
> >   exit(EXIT_FAILURE);
> > 
> > 
> 

I agree these ones are ok, though.

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


signature.asc
Description: PGP signature

Re: [PATCH v3 4/4] block: Mark 'block_resize' as coroutine

Kevin Wolf  writes:

> Am 16.01.2020 um 16:13 hat Markus Armbruster geschrieben:
>> Kevin Wolf  writes:
>> 
>> > Am 16.01.2020 um 10:45 hat Markus Armbruster geschrieben:
>> >> Kevin Wolf  writes:
>> >> > block_resize is safe to run in a coroutine, so use it as an example for
>> >> > the new 'coroutine': true annotation in the QAPI schema.
>> >> >
>> >> > Signed-off-by: Kevin Wolf 
>> >> > Reviewed-by: Marc-André Lureau 
>> >
>> >> > diff --git a/blockdev.c b/blockdev.c
>> >> > index 8e029e9c01..b5e5d1e072 100644
>> >> > --- a/blockdev.c
>> >> > +++ b/blockdev.c
>> >> > @@ -3161,9 +3161,9 @@ void hmp_drive_del(Monitor *mon, const QDict 
>> >> > *qdict)
>> >> >  aio_context_release(aio_context);
>> >> >  }
>> >> >  
>> >> > -void qmp_block_resize(bool has_device, const char *device,
>> >> > -  bool has_node_name, const char *node_name,
>> >> > -  int64_t size, Error **errp)
>> >> > +void coroutine_fn qmp_block_resize(bool has_device, const char *device,
>> >> > +   bool has_node_name, const char 
>> >> > *node_name,
>> >> > +   int64_t size, Error **errp)
>> >> >  {
>> >> >  Error *local_err = NULL;
>> >> >  BlockBackend *blk = NULL;
>> >> 
>> >> Pardon my ignorant question: what exactly makes a function a
>> >> coroutine_fn?
>> >
>> > When Stefan requested adding the coroutine_fn marker, it seemed to make
>> > sense to me because the QMP dispatcher will always call it from
>> > coroutine context now, and being always run in coroutine context makes a
>> > function a coroutine_fn.
>> >
>> > However, it's also called from hmp_block_resize(), so at least for now
>> > coroutine_fn is actually wrong.
>> 
>> This answers the question when we mark a function a coroutine_fn.  I
>> meant to ask what conditions the function itself must satisfy to be
>> eligible for this mark.
>
> The requirement is actually not about the function itself, it's about
> the callers, as stated above.
>
> But being a coroutine_fn allows the function to call other functions
> that only work in coroutine context (other coroutine_fns). In the end
> the reason why a function only works in coroutine context is usually
> that it (or any other coroutine_fns called by it) could yield, which
> obviously doesn't work outside of coroutine contest.

Thanks.

I think "being always run in coroutine context makes a function a
coroutine_fn" is inaccurate.  It's "calling a coroutine_fn without
switching to coroutine context first when not already in coroutine
context".  The induction terminates at basic coroutine_fn like
qemu_coroutine_yield().

Pertinent:

/**
 * Return whether or not currently inside a coroutine
 *
 * This can be used to write functions that work both when in coroutine 
context
 * and when not in coroutine context.  Note that such functions cannot use 
the
 * coroutine_fn annotation since they work outside coroutine context.
 */
bool qemu_in_coroutine(void);

For qmp_block_resize(), it's used like this, in bdrv_truncate():

if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
bdrv_truncate_co_entry(&tco);
} else {
co = qemu_coroutine_create(bdrv_truncate_co_entry, &tco);
bdrv_coroutine_enter(child->bs, co);
BDRV_POLL_WHILE(child->bs, tco.ret == NOT_DONE);
}

where bdrv_truncate_co_entry() is a coroutine_fn.

[PATCH v1] vnc: fix VNC artifacts

2020-01-16 Thread Cameron Esfahani via

Remove VNC optimization to reencode framebuffer update as raw if it's
smaller than the default encoding.  QEMU's implementation was naive and
didn't account for the ZLIB z_stream mutating with each compression.  Just
saving and restoring the output buffer offset wasn't sufficient to "rewind"
the previous encoding.  Considering that ZRLE is never larger than raw and
even though ZLIB can occasionally be fractionally larger than raw, the
overhead of implementing this optimization correctly isn't worth it.

In my investigation, ZRLE always compresses better than ZLIB so
prioritize ZRLE over ZLIB, even if the client hints that ZLIB is
preferred.

Fixes:  ("vnc: allow fall back to RAW encoding")
Signed-off-by: Cameron Esfahani 
---
 ui/vnc-enc-zrle.c |  4 ++--
 ui/vnc.c  | 30 +++---
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/ui/vnc-enc-zrle.c b/ui/vnc-enc-zrle.c
index 17fd28a2e2..b4f71e32cf 100644
--- a/ui/vnc-enc-zrle.c
+++ b/ui/vnc-enc-zrle.c
@@ -98,8 +98,8 @@ static int zrle_compress_data(VncState *vs, int level)
 /* set pointers */
 zstream->next_in = vs->zrle->zrle.buffer;
 zstream->avail_in = vs->zrle->zrle.offset;
-zstream->next_out = vs->zrle->zlib.buffer + vs->zrle->zlib.offset;
-zstream->avail_out = vs->zrle->zlib.capacity - vs->zrle->zlib.offset;
+zstream->next_out = vs->zrle->zlib.buffer;
+zstream->avail_out = vs->zrle->zlib.capacity;
 zstream->data_type = Z_BINARY;
 
 /* start encoding */
diff --git a/ui/vnc.c b/ui/vnc.c
index 4100d6e404..f085e1b747 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -898,8 +898,6 @@ int vnc_raw_send_framebuffer_update(VncState *vs, int x, 
int y, int w, int h)
 int vnc_send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
 {
 int n = 0;
-bool encode_raw = false;
-size_t saved_offs = vs->output.offset;
 
 switch(vs->vnc_encoding) {
 case VNC_ENCODING_ZLIB:
@@ -922,24 +920,11 @@ int vnc_send_framebuffer_update(VncState *vs, int x, int 
y, int w, int h)
 n = vnc_zywrle_send_framebuffer_update(vs, x, y, w, h);
 break;
 default:
-encode_raw = true;
+vnc_framebuffer_update(vs, x, y, w, h, VNC_ENCODING_RAW);
+n = vnc_raw_send_framebuffer_update(vs, x, y, w, h);
 break;
 }
 
-/* If the client has the same pixel format as our internal buffer and
- * a RAW encoding would need less space fall back to RAW encoding to
- * save bandwidth and processing power in the client. */
-if (!encode_raw && vs->write_pixels == vnc_write_pixels_copy &&
-12 + h * w * VNC_SERVER_FB_BYTES <= (vs->output.offset - saved_offs)) {
-vs->output.offset = saved_offs;
-encode_raw = true;
-}
-
-if (encode_raw) {
-vnc_framebuffer_update(vs, x, y, w, h, VNC_ENCODING_RAW);
-n = vnc_raw_send_framebuffer_update(vs, x, y, w, h);
-}
-
 return n;
 }
 
@@ -2087,8 +2072,15 @@ static void set_encodings(VncState *vs, int32_t 
*encodings, size_t n_encodings)
 break;
 #endif
 case VNC_ENCODING_ZLIB:
-vs->features |= VNC_FEATURE_ZLIB_MASK;
-vs->vnc_encoding = enc;
+/*
+ * VNC_ENCODING_ZRLE compresses better than VNC_ENCODING_ZLIB.
+ * So prioritize ZRLE, even if the client hints that it prefers
+ * ZLIB.
+ */
+if ((vs->features & VNC_FEATURE_ZRLE_MASK) == 0) {
+vs->features |= VNC_FEATURE_ZLIB_MASK;
+vs->vnc_encoding = enc;
+}
 break;
 case VNC_ENCODING_ZRLE:
 vs->features |= VNC_FEATURE_ZRLE_MASK;
-- 
2.24.0

[Bug 1860056] Re: mips binaries segfault

Could you attach the version of g++ and qemu? In other words, can you
capture the output of:

mips-linux-gnu-g++ --version

and

qemu-mips --version

?

Does the problem happen if you compile with "-static" option?

Yours,
Aleksandar


** Tags added: mips

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

Title:
  mips binaries segfault

Status in QEMU:
  New

Bug description:
  Hello World appears to segfault with qemu mips, on a Debian 10.0.0
  Buster amd64 host.

  Example:

  
  $ cat mips/test/hello.cpp 
  #include 
  using std::cout;

  int main() {
  cout << "Hello World!\n";
  return 0;
  }

  $ mips-linux-gnu-g++ -o hello hello.cpp && ./hello
  qemu: uncaught target signal 11 (Segmentation fault) - core dumped

  Note that 64-bit MIPS and little endian 32-bit MIPS qemu work fine.
  The problem is limited to big endian 32-bit MIPS.

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

[PATCH 4/4] linux-user: Fix some constants in remaining termbits.h

From: Aleksandar Markovic 

Some constants were defined in terms of host, instead of target,
as they should be.

Signed-off-by: Aleksandar Markovic 
---
 linux-user/aarch64/termbits.h|  4 ++--
 linux-user/arm/termbits.h|  4 ++--
 linux-user/cris/termbits.h   |  4 ++--
 linux-user/hppa/termbits.h   |  4 ++--
 linux-user/i386/termbits.h   |  4 ++--
 linux-user/m68k/termbits.h   |  4 ++--
 linux-user/microblaze/termbits.h |  4 ++--
 linux-user/nios2/termbits.h  |  4 ++--
 linux-user/openrisc/termbits.h   | 14 +++---
 linux-user/ppc/termbits.h|  4 ++--
 linux-user/riscv/termbits.h  |  4 ++--
 linux-user/s390x/termbits.h  | 26 +++---
 linux-user/sh4/termbits.h|  4 ++--
 linux-user/sparc/termbits.h  |  4 ++--
 linux-user/sparc64/termbits.h|  4 ++--
 linux-user/tilegx/termbits.h | 12 
 linux-user/x86_64/termbits.h | 26 +++---
 17 files changed, 75 insertions(+), 55 deletions(-)

diff --git a/linux-user/aarch64/termbits.h b/linux-user/aarch64/termbits.h
index 0ab448d..998fc1d 100644
--- a/linux-user/aarch64/termbits.h
+++ b/linux-user/aarch64/termbits.h
@@ -83,8 +83,8 @@ struct target_termios {
 #define  TARGET_B9600  015
 #define  TARGET_B19200 016
 #define  TARGET_B38400 017
-#define TARGET_EXTA B19200
-#define TARGET_EXTB B38400
+#define TARGET_EXTATARGET_B19200
+#define TARGET_EXTBTARGET_B38400
 #define TARGET_CSIZE   060
 #define   TARGET_CS5   000
 #define   TARGET_CS6   020
diff --git a/linux-user/arm/termbits.h b/linux-user/arm/termbits.h
index e555cff..7170b8a 100644
--- a/linux-user/arm/termbits.h
+++ b/linux-user/arm/termbits.h
@@ -83,8 +83,8 @@ struct target_termios {
 #define  TARGET_B9600  015
 #define  TARGET_B19200 016
 #define  TARGET_B38400 017
-#define TARGET_EXTA B19200
-#define TARGET_EXTB B38400
+#define TARGET_EXTATARGET_B19200
+#define TARGET_EXTBTARGET_B38400
 #define TARGET_CSIZE   060
 #define   TARGET_CS5   000
 #define   TARGET_CS6   020
diff --git a/linux-user/cris/termbits.h b/linux-user/cris/termbits.h
index 475ee70..76d5ed0 100644
--- a/linux-user/cris/termbits.h
+++ b/linux-user/cris/termbits.h
@@ -81,8 +81,8 @@ struct target_termios {
 #define  TARGET_B9600  015
 #define  TARGET_B19200 016
 #define  TARGET_B38400 017
-#define TARGET_EXTA B19200
-#define TARGET_EXTB B38400
+#define TARGET_EXTATARGET_B19200
+#define TARGET_EXTBTARGET_B38400
 #define TARGET_CSIZE   060
 #define   TARGET_CS5   000
 #define   TARGET_CS6   020
diff --git a/linux-user/hppa/termbits.h b/linux-user/hppa/termbits.h
index 8fba839..3094710 100644
--- a/linux-user/hppa/termbits.h
+++ b/linux-user/hppa/termbits.h
@@ -82,8 +82,8 @@ struct target_termios {
 #define  TARGET_B9600  015
 #define  TARGET_B19200 016
 #define  TARGET_B38400 017
-#define TARGET_EXTA B19200
-#define TARGET_EXTB B38400
+#define TARGET_EXTATARGET_B19200
+#define TARGET_EXTBTARGET_B38400
 #define TARGET_CSIZE   060
 #define   TARGET_CS5   000
 #define   TARGET_CS6   020
diff --git a/linux-user/i386/termbits.h b/linux-user/i386/termbits.h
index 88264bb..3b16977 100644
--- a/linux-user/i386/termbits.h
+++ b/linux-user/i386/termbits.h
@@ -82,8 +82,8 @@ struct target_termios {
 #define  TARGET_B9600  015
 #define  TARGET_B19200 016
 #define  TARGET_B38400 017
-#define TARGET_EXTA B19200
-#define TARGET_EXTB B38400
+#define TARGET_EXTATARGET_B19200
+#define TARGET_EXTBTARGET_B38400
 #define TARGET_CSIZE   060
 #define   TARGET_CS5   000
 #define   TARGET_CS6   020
diff --git a/linux-user/m68k/termbits.h b/linux-user/m68k/termbits.h
index 23840aa..f3ae025 100644
--- a/linux-user/m68k/termbits.h
+++ b/linux-user/m68k/termbits.h
@@ -83,8 +83,8 @@ struct target_termios {
 #define  TARGET_B9600  015
 #define  TARGET_B19200 016
 #define  TARGET_B38400 017
-#define TARGET_EXTA B19200
-#define TARGET_EXTB B38400
+#define TARGET_EXTATARGET_B19200
+#define TARGET_EXTBTARGET_B38400
 #define TARGET_CSIZE   060
 #define   TARGET_CS5   000
 #define   TARGET_CS6   020
diff --git a/linux-user/microblaze/termbits.h b/linux-user/microblaze/termbits.h
index 17db8a4..7697736 100644
--- a/linux-user/microblaze/termbits.h
+++ b/linux-user/microblaze/termbits.h
@@ -81,8 +81,8 @@ struct target_termios {
 #define  TARGET_B9600  015
 #define  TARGET_B19200 016
 #define  TARGET_B38400 017
-#define TARGET_EXTA B19200
-#define TARGET_EXTB B38400
+#define TARGET_EXTATARGET_B19200
+#define TARGET_EXTBTARGET_B38400
 #define TARGET_CSIZE   060
 #define   TARGET_CS5   000
 #define   TARGET_CS6   020
diff --git a/linux-user/nios2/termbits.h b/linux-user/nios2/termbits.h
index 425a2fe..269ab59 100644
--- a/linux-user/nios2/termbits.h
+++ b/linux-user/nios2/termbits.h
@@ -83,8 +83,8 @@ struct target_termios {
 #define  TARGET_B9600

[PATCH 2/4] linux-user: mips: Synchronize termbits.h with kernel

From: Aleksandar Markovic 

Synchronize all elements of mips' termbits.h with kernel and
make sure that all applicable macros and other definitions are
expressed in terms of target, not the host.

Signed-off-by: Aleksandar Markovic 
---
 linux-user/mips/termbits.h | 140 -
 1 file changed, 89 insertions(+), 51 deletions(-)

diff --git a/linux-user/mips/termbits.h b/linux-user/mips/termbits.h
index 3287cf6..79a9b9b 100644
--- a/linux-user/mips/termbits.h
+++ b/linux-user/mips/termbits.h
@@ -3,33 +3,89 @@
 #ifndef LINUX_USER_MIPS_TERMBITS_H
 #define LINUX_USER_MIPS_TERMBITS_H
 
-#define TARGET_NCCS 23
+typedef unsigned char   target_cc_t;
+typedef unsigned inttarget_speed_t;
+typedef unsigned inttarget_tcflag_t;
 
+/*
+ * The ABI says nothing about NCC but seems to use NCCS as
+ * replacement for it in struct termio
+ */
+#define TARGET_NCCS 23
 struct target_termios {
-unsigned int c_iflag;   /* input mode flags */
-unsigned int c_oflag;   /* output mode flags */
-unsigned int c_cflag;   /* control mode flags */
-unsigned int c_lflag;   /* local mode flags */
-unsigned char c_line;/* line discipline */
-unsigned char c_cc[TARGET_NCCS];/* control characters */
+target_tcflag_t c_iflag;/* input mode flags */
+target_tcflag_t c_oflag;/* output mode flags */
+target_tcflag_t c_cflag;/* control mode flags */
+target_tcflag_t c_lflag;/* local mode flags */
+target_cc_t c_line; /* line discipline */
+target_cc_t c_cc[TARGET_NCCS];  /* control characters */
+};
+
+struct target_termios2 {
+target_tcflag_t c_iflag;/* input mode flags */
+target_tcflag_t c_oflag;/* output mode flags */
+target_tcflag_t c_cflag;/* control mode flags */
+target_tcflag_t c_lflag;/* local mode flags */
+target_cc_t c_line; /* line discipline */
+target_cc_t c_cc[TARGET_NCCS];  /* control characters */
+target_speed_t c_ispeed;/* input speed */
+target_speed_t c_ospeed;/* output speed */
+};
+
+struct target_ktermios {
+target_tcflag_t c_iflag;/* input mode flags */
+target_tcflag_t c_oflag;/* output mode flags */
+target_tcflag_t c_cflag;/* control mode flags */
+target_tcflag_t c_lflag;/* local mode flags */
+target_cc_t c_line; /* line discipline */
+target_cc_t c_cc[TARGET_NCCS];  /* control characters */
+target_speed_t c_ispeed;/* input speed */
+target_speed_t c_ospeed;/* output speed */
 };
 
+/* c_cc character offsets */
+#define TARGET_VINTR0   /* Interrupt character [ISIG].  */
+#define TARGET_VQUIT1   /* Quit character [ISIG].  */
+#define TARGET_VERASE   2   /* Erase character [ICANON].  */
+#define TARGET_VKILL3   /* Kill-line character [ICANON].  */
+#define TARGET_VMIN 4   /* Minimum number of bytes read at once */
+#define TARGET_VTIME5   /* Time-out value (tenths of a second) */
+#define TARGET_VEOL26   /* Second EOL character [ICANON].  */
+#define TARGET_VSWTC7   /* ??? */
+#define TARGET_VSWTCH   VSWTC
+#define TARGET_VSTART   8   /* Start (X-ON) character [IXON, IXOFF].  */
+#define TARGET_VSTOP9   /* Stop (X-OFF) character [IXON, IXOFF].  */
+#define TARGET_VSUSP10  /* Suspend character [ISIG].  */
+
+#if 0
+/*
+ * VDSUSP is not supported
+ */
+#define TARGET_VDSUSP  11   /* Delayed suspend character [ISIG].  */
+#endif
+#define TARGET_VREPRINT 12  /* Reprint-line character [ICANON].  */
+#define TARGET_VDISCARD 13  /* Discard character [IEXTEN].  */
+#define TARGET_VWERASE  14  /* Word-erase character [ICANON].  */
+#define TARGET_VLNEXT   15  /* Literal-next character [IEXTEN].  */
+#define TARGET_VEOF 16  /* End-of-file character [ICANON].  */
+#define TARGET_VEOL 17  /* End-of-line character [ICANON].  */
+
 /* c_iflag bits */
-#define TARGET_IGNBRK  001
-#define TARGET_BRKINT  002
-#define TARGET_IGNPAR  004
-#define TARGET_PARMRK  010
-#define TARGET_INPCK   020
-#define TARGET_ISTRIP  040
-#define TARGET_INLCR   100
-#define TARGET_IGNCR   200
-#define TARGET_ICRNL   400
-#define TARGET_IUCLC   0001000
-#define TARGET_IXON0002000
-#define TARGET_IXANY   0004000
-#define TARGET_IXOFF   001
-#define TARGET_IMAXBEL 002
-#define TARGET_IUTF8   004
+#define TARGET_IGNBRK  001  /* Ignore break condition.  */
+#define TARGET_BRKINT  002  /* Signal interrupt on break.  */
+#define TARGET_IGNPAR  004  /* Ignore characters with parity errors.  */
+#define TARGET_PARMRK  010  /* Mark parity and framing errors.  */
+#define TARGET_INPCK   020  /*

[PATCH 3/4] linux-user: xtensa: Fix some constants in termbits.h

From: Aleksandar Markovic 

Some constants were defined in terms of host, instead of target,
as they should be.

Some additional trivial changes in this patch were forced by
checkpatch.pl.

Reviewed-by: Max Filippov 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/xtensa/termbits.h | 207 +++
 1 file changed, 113 insertions(+), 94 deletions(-)

diff --git a/linux-user/xtensa/termbits.h b/linux-user/xtensa/termbits.h
index d1e09e6..abc8666 100644
--- a/linux-user/xtensa/termbits.h
+++ b/linux-user/xtensa/termbits.h
@@ -15,40 +15,41 @@
 
 #include 
 
-typedef unsigned char   cc_t;
-typedef unsigned intspeed_t;
-typedef unsigned inttcflag_t;
+typedef unsigned char   target_cc_t;
+typedef unsigned inttarget_speed_t;
+typedef unsigned inttarget_tcflag_t;
 
 #define TARGET_NCCS 19
+
 struct target_termios {
-tcflag_t c_iflag;   /* input mode flags */
-tcflag_t c_oflag;   /* output mode flags */
-tcflag_t c_cflag;   /* control mode flags */
-tcflag_t c_lflag;   /* local mode flags */
-cc_t c_line;/* line discipline */
-cc_t c_cc[TARGET_NCCS]; /* control characters */
+target_tcflag_t c_iflag;   /* input mode flags */
+target_tcflag_t c_oflag;   /* output mode flags */
+target_tcflag_t c_cflag;   /* control mode flags */
+target_tcflag_t c_lflag;   /* local mode flags */
+target_cc_t c_line;/* line discipline */
+target_cc_t c_cc[TARGET_NCCS]; /* control characters */
 };
 
 struct target_termios2 {
-tcflag_t c_iflag;   /* input mode flags */
-tcflag_t c_oflag;   /* output mode flags */
-tcflag_t c_cflag;   /* control mode flags */
-tcflag_t c_lflag;   /* local mode flags */
-cc_t c_line;/* line discipline */
-cc_t c_cc[TARGET_NCCS]; /* control characters */
-speed_t c_ispeed;   /* input speed */
-speed_t c_ospeed;   /* output speed */
+target_tcflag_t c_iflag;   /* input mode flags */
+target_tcflag_t c_oflag;   /* output mode flags */
+target_tcflag_t c_cflag;   /* control mode flags */
+target_tcflag_t c_lflag;   /* local mode flags */
+target_cc_t c_line;/* line discipline */
+target_cc_t c_cc[TARGET_NCCS]; /* control characters */
+target_speed_t c_ispeed;   /* input speed */
+target_speed_t c_ospeed;   /* output speed */
 };
 
 struct target_ktermios {
-tcflag_t c_iflag;   /* input mode flags */
-tcflag_t c_oflag;   /* output mode flags */
-tcflag_t c_cflag;   /* control mode flags */
-tcflag_t c_lflag;   /* local mode flags */
-cc_t c_line;/* line discipline */
-cc_t c_cc[TARGET_NCCS]; /* control characters */
-speed_t c_ispeed;   /* input speed */
-speed_t c_ospeed;   /* output speed */
+target_tcflag_t c_iflag;   /* input mode flags */
+target_tcflag_t c_oflag;   /* output mode flags */
+target_tcflag_t c_cflag;   /* control mode flags */
+target_tcflag_t c_lflag;   /* local mode flags */
+target_cc_t c_line;/* line discipline */
+target_cc_t c_cc[TARGET_NCCS]; /* control characters */
+target_speed_t c_ispeed;   /* input speed */
+target_speed_t c_ospeed;   /* output speed */
 };
 
 /* c_cc characters */
@@ -142,8 +143,8 @@ struct target_ktermios {
 #define  TARGET_B9600   015
 #define  TARGET_B19200  016
 #define  TARGET_B38400  017
-#define TARGET_EXTA B19200
-#define TARGET_EXTB B38400
+#define TARGET_EXTA TARGET_B19200
+#define TARGET_EXTB TARGET_B38400
 #define TARGET_CSIZE060
 #define   TARGET_CS5000
 #define   TARGET_CS6020
@@ -217,13 +218,13 @@ struct target_ktermios {
 
 /* from arch/xtensa/include/uapi/asm/ioctls.h */
 
-#define TARGET_FIOCLEX _IO('f', 1)
-#define TARGET_FIONCLEX_IO('f', 2)
-#define TARGET_FIOASYNC_IOW('f', 125, int)
-#define TARGET_FIONBIO _IOW('f', 126, int)
-#define TARGET_FIONREAD_IOR('f', 127, int)
-#define TARGET_TIOCINQ FIONREAD
-#define TARGET_FIOQSIZE_IOR('f', 128, loff_t)
+#define TARGET_FIOCLEX TARGET_IO('f', 1)
+#define TARGET_FIONCLEXTARGET_IO('f', 2)
+#define TARGET_FIOASYNCTARGET_IOW('f', 125, int)
+#define TARGET_FIONBIO TARGET_IOW('f', 126, int)
+#define TARGET_FIONREADTARGET_IOR('f', 127, int)
+#define TARGET_TIOCINQ TARGET_FIONREAD
+#define TARGET_FIOQSIZETARGET_IOR('f', 128, loff_t)
 
 #define TARGET_TCGETS  0x5401
 #define TARGET_TCSETS  0x5402
@@ -235,28 +236,28 @@ struct target_ktermios {
 #define TARGET_TCSETAW 0x40127419  /* _IOW('t', 25, struct termio) */
 #define TARGET_TCSETAF 0x4012741C  /* _IOW('t', 28, struct termio) */
 
-#define TARGET_TCSBRK  _IO('t', 29)
-#define TARGET_TCXONC  _IO('t', 30)
-#define TARGET_TCFLSH  _IO('t', 31)
+#define TARGET_TCSBRK  TARGET_IO('t', 29)
+#define TARGET_TC

[PATCH 1/4] linux-user: alpha: Synchronize termbits.h with kernel

From: Aleksandar Markovic 

Synchronize all elements of alpha's termbits.h with kernel and
make sure that all applicable macros and other definitions are
expressed in terms of target, not the host.

Signed-off-by: Aleksandar Markovic 
---
 linux-user/alpha/termbits.h | 82 +
 1 file changed, 76 insertions(+), 6 deletions(-)

diff --git a/linux-user/alpha/termbits.h b/linux-user/alpha/termbits.h
index a714251..d15f26e 100644
--- a/linux-user/alpha/termbits.h
+++ b/linux-user/alpha/termbits.h
@@ -17,6 +17,32 @@ struct target_termios {
target_speed_t c_ospeed;/* output speed */
 };
 
+/* Alpha has identical termios and termios2 */
+
+struct target_termios2 {
+target_tcflag_t c_iflag;/* input mode flags */
+target_tcflag_t c_oflag;/* output mode flags */
+target_tcflag_t c_cflag;/* control mode flags */
+target_tcflag_t c_lflag;/* local mode flags */
+target_cc_t c_cc[TARGET_NCCS];  /* control characters */
+target_cc_t c_line; /* line discipline (== c_cc[19]) */
+target_speed_t c_ispeed;/* input speed */
+target_speed_t c_ospeed;/* output speed */
+};
+
+/* Alpha has matching termios and ktermios */
+
+struct target_ktermios {
+target_tcflag_t c_iflag;/* input mode flags */
+target_tcflag_t c_oflag;/* output mode flags */
+target_tcflag_t c_cflag;/* control mode flags */
+target_tcflag_t c_lflag;/* local mode flags */
+target_cc_t c_cc[TARGET_NCCS];  /* control characters */
+target_cc_t c_line; /* line discipline (== c_cc[19]) */
+target_speed_t c_ispeed;/* input speed */
+target_speed_t c_ospeed;/* output speed */
+};
+
 /* c_cc characters */
 #define TARGET_VEOF 0
 #define TARGET_VEOL 1
@@ -88,7 +114,11 @@ struct target_termios {
 #define TARGET_VTDLY   0020
 #define   TARGET_VT0   
 #define   TARGET_VT1   0020
-#define TARGET_XTABS   0100 /* Hmm.. Linux/i386 considers this part of 
TABDLY.. */
+/*
+ * Should be equivalent to TAB3, see description of TAB3 in
+ * POSIX.1-2008, Ch. 11.2.3 "Output Modes"
+ */
+#define TARGET_XTABSTARGET_TAB3
 
 /* c_cflag bit meaning */
 #define TARGET_CBAUD   037
@@ -108,8 +138,8 @@ struct target_termios {
 #define  TARGET_B9600  015
 #define  TARGET_B19200 016
 #define  TARGET_B38400 017
-#define TARGET_EXTA B19200
-#define TARGET_EXTB B38400
+#define TARGET_EXTA TARGET_B19200
+#define TARGET_EXTB TARGET_B38400
 #define TARGET_CBAUDEX 000
 #define  TARGET_B57600   00020
 #define  TARGET_B115200  00021
@@ -143,6 +173,9 @@ struct target_termios {
 #define TARGET_CMSPAR0100  /* mark or space (stick) parity 
*/
 #define TARGET_CRTSCTS   0200  /* flow control */
 
+#define TARGET_CIBAUD   0760
+#define TARGET_IBSHIFT  16
+
 /* c_lflag bits */
 #define TARGET_ISIG0x0080
 #define TARGET_ICANON  0x0100
@@ -159,13 +192,30 @@ struct target_termios {
 #define TARGET_FLUSHO  0x0080
 #define TARGET_PENDIN  0x2000
 #define TARGET_IEXTEN  0x0400
+#define TARGET_EXTPROC  0x1000
+
+/* Values for the ACTION argument to `tcflow'.  */
+#define TCOOFF  0
+#define TCOON   1
+#define TCIOFF  2
+#define TCION   3
+
+/* Values for the QUEUE_SELECTOR argument to `tcflush'.  */
+#define TCIFLUSH0
+#define TCOFLUSH1
+#define TCIOFLUSH   2
+
+/* Values for the OPTIONAL_ACTIONS argument to `tcsetattr'.  */
+#define TCSANOW 0
+#define TCSADRAIN   1
+#define TCSAFLUSH   2
 
 #define TARGET_FIOCLEX TARGET_IO('f', 1)
 #define TARGET_FIONCLEXTARGET_IO('f', 2)
 #define TARGET_FIOASYNCTARGET_IOW('f', 125, int)
 #define TARGET_FIONBIO TARGET_IOW('f', 126, int)
 #define TARGET_FIONREADTARGET_IOR('f', 127, int)
-#define TARGET_TIOCINQ FIONREAD
+#define TARGET_TIOCINQ  TARGET_FIONREAD
 #define TARGET_FIOQSIZETARGET_IOR('f', 128, loff_t)
 
 #define TARGET_TIOCGETPTARGET_IOR('t', 8, struct target_sgttyb)
@@ -188,6 +238,11 @@ struct target_termios {
 #define TARGET_TCXONC  TARGET_IO('t', 30)
 #define TARGET_TCFLSH  TARGET_IO('t', 31)
 
+#define TARGET_TCGETS2  TARGET_IOR('T', 42, struct target_termios2)
+#define TARGET_TCSETS2  TARGET_IOW('T', 43, struct target_termios2)
+#define TARGET_TCSETSW2 TARGET_IOW('T', 44, struct target_termios2)
+#define TARGET_TCSETSF2 TARGET_IOW('T', 45, struct target_termios2)
+
 #define TARGET_TIOCSWINSZ  TARGET_IOW('t', 103, struct target_winsize)
 #define TARGET_TIOCGWINSZ  TARGET_IOR('t', 104, struct target_winsize)
 #defineTARGET_TIOCSTARTTARGET_IO('t', 110) /* 
start output, like ^Q */
@@ -217,8 +272,8 @@ struct target_termios {
 # define TARG

[PATCH 0/4] linux-user: Fix some issues in termbits.h files

From: Aleksandar Markovic 

This series is a spin-off of v5 of earlier series "linux-user: Misc
patches for 5.0", that became too large to manage. I will submit the
rest of that large series separately.

Files linux-user//termbits.h seem to be in a very bad shape:
unsynchronized with kernel, containing wrong elements expressed in
terms of host instead of target, many being updated wrt kernel
content at various times, and on top of that all contain visually
very ugly combinations of tabs and spaces.

This series attempts to fix great majority of issues in termbits.

Alpha's and mips' termbits.h were in the worst shape, missing large
bits and pieces, and for them as complete as possible synchronization
with kernel code is done - this constitutes the first two patches.

Xtensa's termbits.h contained the most elements wrongly expressed in
terms of host instead of target, and that is the reason the changes
in this file are placed in a separate, third, patch. Previous "R-B"
given by Max Filippov was transferred to this patch only.

The fourth patch fixes remaining elements wrongly expressed in
terms of host instead of target.

As an additional note, structures "serial_iso7816" and "serial_rs485"
(at times mentioned as the third argument of certain ioctls) are
platform-independant in kernel, and do not need "target_" variant
in QEMU. Also, structure "winsize" (also appearing as the third
ioctl's argument at times) is defined at multiple places in kernel
(for several architectures) in kernel, but all such definitions are
identical, and, therefore, it also does not need "target_" variant
in QEMU.

A checkpatch warning related to "#if 0" in patch 2 is benign, and
should be ignored.

Aleksandar Markovic (4):
  linux-user: alpha: Synchronize termbits.h with kernel
  linux-user: mips: Synchronize termbits.h with kernel
  linux-user: xtensa: Fix some constants in termbits.h
  linux-user: Fix some constants in remaining termbits.h

 linux-user/aarch64/termbits.h|   4 +-
 linux-user/alpha/termbits.h  |  82 ++--
 linux-user/arm/termbits.h|   4 +-
 linux-user/cris/termbits.h   |   4 +-
 linux-user/hppa/termbits.h   |   4 +-
 linux-user/i386/termbits.h   |   4 +-
 linux-user/m68k/termbits.h   |   4 +-
 linux-user/microblaze/termbits.h |   4 +-
 linux-user/mips/termbits.h   | 140 --
 linux-user/nios2/termbits.h  |   4 +-
 linux-user/openrisc/termbits.h   |  14 +--
 linux-user/ppc/termbits.h|   4 +-
 linux-user/riscv/termbits.h  |   4 +-
 linux-user/s390x/termbits.h  |  26 ++---
 linux-user/sh4/termbits.h|   4 +-
 linux-user/sparc/termbits.h  |   4 +-
 linux-user/sparc64/termbits.h|   4 +-
 linux-user/tilegx/termbits.h |  12 ++-
 linux-user/x86_64/termbits.h |  26 +++--
 linux-user/xtensa/termbits.h | 207 +--
 20 files changed, 353 insertions(+), 206 deletions(-)

-- 
2.7.4

Re: [PATCH 2/2] arm/virt/acpi: remove _ADR from devices identified by _HID

2020-01-16 Thread Guoheyi




在 2020/1/16 20:24, Peter Maydell 写道:

On Wed, 15 Jan 2020 at 10:55, Michael S. Tsirkin  wrote:

Here's a hopefully better patch. Peter does this address the issue?

Signed-off-by: Michael S. Tsirkin 


diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index f1ac2d7e96..3ab4872bd7 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -16,7 +16,10 @@
   * 1. add empty files for new tables, if any, under tests/data/acpi
   * 2. list any changed files in tests/bios-tables-test-allowed-diff.h
   * 3. commit the above *before* making changes that affect the tables
- * Maintainer:
+ *
+ * Contributor or ACPI Maintainer (steps 4-7 need to be redone to resolve 
conflicts
+ * in binary commit created in step 6):
+ *
   * After 1-3 above tests will pass but ignore differences with the expected 
files.
   * You will also notice that tests/bios-tables-test-allowed-diff.h lists
   * a bunch of files. This is your hint that you need to do the below:
@@ -28,13 +31,23 @@
   * output. If not - disassemble them yourself in any way you like.
   * Look at the differences - make sure they make sense and match what the
   * changes you are merging are supposed to do.
+ * Save the changes, preferably in form of ASL diff for the commit log in
+ * step 6.
   *
   * 5. From build directory, run:
   *  $(SRC_PATH)/tests/data/acpi/rebuild-expected-aml.sh
- * 6. Now commit any changes.
- * 7. Before doing a pull request, make sure 
tests/bios-tables-test-allowed-diff.h
- *is empty - this will ensure following changes to ACPI tables will
- *be noticed.
+ * 6. Now commit any changes to the expected binary, include diff from step 4
+ *in commit log.
+ * 7. Before sending patches to the list (Contributor)
+ *or before doing a pull request (Maintainer), make sure
+ *tests/bios-tables-test-allowed-diff.h is empty - this will ensure
+ *following changes to ACPI tables will be noticed.
+ *
+ * The resulting patchset/pull request then looks like this:
+ * - patch 1: list changed files in tests/bios-tables-test-allowed-diff.h.
+ * - patches 2 - n: real changes, may contain multiple patches.
+ * - patch n + 1: update golden master binaries and empty


Though I drafted the inital text, "2 - n" seems like a expression "2 
minus n" for myself after a second glance, especially when we have a "n 
+1" just below. Maybe we can use a different notation :)




+ *   tests/bios-tables-test-allowed-diff.h
   */

I think that seems reasonable, but you're the ACPI expert.
As long as the patches on list:
  * can be reviewed by somebody for whether their ACPI changes
are correct, including whether the golden-master changes are right
  * can be applied by a maintainer without having to do anything
special
  * don't break bisection

then I'm happy. It sounds like those steps will work for that.


diff --git a/roms/seabios b/roms/seabios
index f21b5a4aeb..c9ba5276e3 16
--- a/roms/seabios
+++ b/roms/seabios
@@ -1 +1 @@
-Subproject commit f21b5a4aeb020f2a5e2c6503f906a9349dd2f069
+Subproject commit c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d

You have a stray submodule update in your patch, though :-)


A file config.mak will be generated in roms/seabios after building qemu, 
and we may probably add it in our commit by "git commit -a" :)


Thanks,

Heyi



thanks
-- PMM

.

Re: [PATCH 2/2] arm/virt/acpi: remove _ADR from devices identified by _HID

2020-01-16 Thread Guoheyi




在 2020/1/16 21:25, Igor Mammedov 写道:

On Thu, 16 Jan 2020 19:56:19 +0800
Guoheyi  wrote:


在 2020/1/5 20:53, Michael S. Tsirkin 写道:

On Sun, Jan 05, 2020 at 07:34:01AM -0500, Michael S. Tsirkin wrote:

On Thu, Dec 19, 2019 at 02:47:59PM +0800, Heyi Guo wrote:

According to ACPI spec, _ADR should be used for device which is on a
bus that has a standard enumeration algorithm. It does not make sense
to have a _ADR object for devices which already have _HID and will be
enumerated by OSPM.

Signed-off-by: Heyi Guo 

Are you sure? I would think this depends on the ID and the device
really. E.g. PCI devices all are expected to have _ADR and some of them
have a _HID.

To clarify I am not commenting on patches.
The spec says this:
6.1.5 _HID (Hardware ID)

This object is used to supply OSPM with the device’s PNP ID or ACPI ID. 
1

When describing a platform, use of any _HID objects is optional. 
However, a _HID object must be

used to describe any device that will be enumerated by OSPM. OSPM only 
enumerates a device

when no bus enumerator can detect the device ID. For example, devices 
on an ISA bus are

enumerated by OSPM. Use the _ADR object to describe devices enumerated 
by bus enumerators

other than OSPM.


Note: "detect the device ID" not "enumerate the device" which I think
means there's a driver matching this vendor/device ID.

So it seems fine to have _ADR so device is enumerated, and still have
_HID e.g. so ACPI driver can be loaded as fallback if there's
no bus driver.


Note I am not saying the patch itself is not correct.
Maybe these devices are not on any standard bus and that
is why they should not have _ADR? I have not looked.

I am just saying that spec does not seem to imply _HID and _ADR
can't coexist.

More reading on the spec, I found a statement as below
(https://uefi.org/sites/default/files/resources/ACPI_6_3_May16.pdf,
section 6.1, on top of page 343):

I'd replace 'It does not make sense ...' sentence with pointer to spec
and quote below in commit message.


Sure; actually I hadn't found this statement in the spec when making the 
patch :)


Thanks,

Heyi





A device object must contain either an _HID object or an _ADR object,
but should not contain both

[...]


.

[PATCH] target/hppa: Allow, but diagnose, LDCW aligned only mod 4

The PA-RISC 1.1 specification says that LDCW must be aligned mod 16
or the operation is undefined.  However, real hardware only generates
an unaligned access trap for unaligned mod 4.

Match real hardware, but diagnose with GUEST_ERROR a violation of the
specification.

Reported-by: Helge Deller 
Suggested-by: John David Anglin 
Signed-off-by: Richard Henderson 
---

Helge, can you please test this against your failing kernel?
You will of course want to add -D logfile -d guest_errors to
you qemu command-line.


r~

---
 target/hppa/helper.h| 2 ++
 target/hppa/op_helper.c | 9 +
 target/hppa/translate.c | 6 +-
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/target/hppa/helper.h b/target/hppa/helper.h
index 38d834ef6b..2d483aab58 100644
--- a/target/hppa/helper.h
+++ b/target/hppa/helper.h
@@ -17,6 +17,8 @@ DEF_HELPER_FLAGS_3(stby_b_parallel, TCG_CALL_NO_WG, void, 
env, tl, tr)
 DEF_HELPER_FLAGS_3(stby_e, TCG_CALL_NO_WG, void, env, tl, tr)
 DEF_HELPER_FLAGS_3(stby_e_parallel, TCG_CALL_NO_WG, void, env, tl, tr)
 
+DEF_HELPER_FLAGS_1(ldc_check, TCG_CALL_NO_RWG, void, tl)
+
 DEF_HELPER_FLAGS_4(probe, TCG_CALL_NO_WG, tr, env, tl, i32, i32)
 
 DEF_HELPER_FLAGS_1(loaded_fr0, TCG_CALL_NO_RWG, void, env)
diff --git a/target/hppa/op_helper.c b/target/hppa/op_helper.c
index f0516e81f1..345cef2c08 100644
--- a/target/hppa/op_helper.c
+++ b/target/hppa/op_helper.c
@@ -153,6 +153,15 @@ void HELPER(stby_e_parallel)(CPUHPPAState *env, 
target_ulong addr,
 do_stby_e(env, addr, val, true, GETPC());
 }
 
+void HELPER(ldc_check)(target_ulong addr)
+{
+if (unlikely(addr & 0xf)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "Undefined ldc to address unaligned mod 16: "
+  TARGET_FMT_lx "\n", addr);
+}
+}
+
 target_ureg HELPER(probe)(CPUHPPAState *env, target_ulong addr,
   uint32_t level, uint32_t want)
 {
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 2f8d407a82..669381dc1d 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2942,7 +2942,7 @@ static bool trans_st(DisasContext *ctx, arg_ldst *a)
 
 static bool trans_ldc(DisasContext *ctx, arg_ldst *a)
 {
-MemOp mop = MO_TEUL | MO_ALIGN_16 | a->size;
+MemOp mop = MO_TE | MO_ALIGN | a->size;
 TCGv_reg zero, dest, ofs;
 TCGv_tl addr;
 
@@ -2958,8 +2958,12 @@ static bool trans_ldc(DisasContext *ctx, arg_ldst *a)
 
 form_gva(ctx, &addr, &ofs, a->b, a->x, a->scale ? a->size : 0,
  a->disp, a->sp, a->m, ctx->mmu_idx == MMU_PHYS_IDX);
+
+gen_helper_ldc_check(addr);
 zero = tcg_const_reg(0);
 tcg_gen_atomic_xchg_reg(dest, addr, zero, ctx->mmu_idx, mop);
+tcg_temp_free(zero);
+
 if (a->m) {
 save_gpr(ctx, a->b, ofs);
 }
-- 
2.20.1

RE: [PATCH RFC 04/12] migration/rdma: Create multiRDMA migration threads

2020-01-16 Thread fengzhimin

Thanks for your review. I will merge this with multifd.

-Original Message-
From: Juan Quintela [mailto:quint...@redhat.com] 
Sent: Thursday, January 16, 2020 9:25 PM
To: fengzhimin 
Cc: dgilb...@redhat.com; arm...@redhat.com; ebl...@redhat.com; 
qemu-devel@nongnu.org; Zhanghailiang ; 
jemmy858...@gmail.com
Subject: Re: [PATCH RFC 04/12] migration/rdma: Create multiRDMA migration 
threads

Zhimin Feng  wrote:
> From: fengzhimin 
>
> Creation of the RDMA threads, nothing inside yet.
>
> Signed-off-by: fengzhimin 

> ---
>  migration/migration.c |   1 +
>  migration/migration.h |   2 +
>  migration/rdma.c  | 283 ++
>  3 files changed, 286 insertions(+)
>
> diff --git a/migration/migration.c b/migration/migration.c index 
> 5756a4806e..f8d4eb657e 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1546,6 +1546,7 @@ static void migrate_fd_cleanup(MigrationState *s)
>  qemu_mutex_lock_iothread();
>  
>  multifd_save_cleanup();
> +multiRDMA_save_cleanup();

Can we merge this with multifd?


> +typedef struct {
> +/* this fields are not changed once the thread is created */
> +/* channel number */
> +uint8_t id;
> +/* channel thread name */
> +char *name;
> +/* channel thread id */
> +QemuThread thread;
> +/* sem where to wait for more work */
> +QemuSemaphore sem;
> +/* this mutex protects the following parameters */
> +QemuMutex mutex;
> +/* is this channel thread running */
> +bool running;
> +/* should this thread finish */
> +bool quit;
> +}  MultiRDMASendParams;

This is basically the same than MultiFBSendParams, same for the rest.

I would very much preffer not to have two sets of threads that are really 
equivalent.

Thanks, Juan.

RE: [PATCH RFC 01/12] migration: Add multiRDMA capability support

2020-01-16 Thread fengzhimin

Thanks for your review. I will modify its according to your suggestions.

-Original Message-
From: Juan Quintela [mailto:quint...@redhat.com] 
Sent: Thursday, January 16, 2020 9:19 PM
To: Dr. David Alan Gilbert 
Cc: fengzhimin ; arm...@redhat.com; ebl...@redhat.com; 
qemu-devel@nongnu.org; Zhanghailiang ; 
jemmy858...@gmail.com
Subject: Re: [PATCH RFC 01/12] migration: Add multiRDMA capability support

"Dr. David Alan Gilbert"  wrote:
> * Zhimin Feng (fengzhim...@huawei.com) wrote:
>> From: fengzhimin 
>> 
>> Signed-off-by: fengzhimin 
>
> Instead of creating x-multirdma as a capability and the corresponding 
> parameter for the number of channels; it would be better just to use 
> the multifd parameters when used with an rdma transport; as far as I 
> know multifd doesn't work with rdma at the moment, and to the user the 
> idea of multifd over rdma is just the same thing.

I was about to suggest that.  We could setup both capabilities:

multifd + rdma

Re: [PATCH v2 0/3] hw/hppa/machine: Restrict the total memory size to 3GB

On 1/8/20 2:05 PM, Philippe Mathieu-Daudé wrote:
> Following the discussion of Igor's patch "hppa: allow max
> ram size upto 4Gb" [1] I tried to simplify the current
> code so Igor's series doesn't change the CLI with this
> machine.
> 
> v2: Simplify by limiting to 3GB (Helge review)
> 
> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg667903.html
> [2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg669309.html
> 
> Philippe Mathieu-Daudé (3):
>   hw/hppa/machine: Correctly check the firmware is in PDC range
>   hw/hppa/machine: Restrict the total memory size to 3GB
>   hw/hppa/machine: Map the PDC memory region with higher priority
> 
>  hw/hppa/machine.c | 13 ++---
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 

Queued.


r~

[Bug 1860056] [NEW] mips binaries segfault

2020-01-16 Thread mcandre

Public bug reported:

Hello World appears to segfault with qemu mips, on a Debian 10.0.0
Buster amd64 host.

Example:


$ cat mips/test/hello.cpp 
#include 
using std::cout;

int main() {
cout << "Hello World!\n";
return 0;
}

$ mips-linux-gnu-g++ -o hello hello.cpp && ./hello
qemu: uncaught target signal 11 (Segmentation fault) - core dumped

Note that 64-bit MIPS and little endian 32-bit MIPS qemu work fine. The
problem is limited to big endian 32-bit MIPS.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  mips binaries segfault

Status in QEMU:
  New

Bug description:
  Hello World appears to segfault with qemu mips, on a Debian 10.0.0
  Buster amd64 host.

  Example:

  
  $ cat mips/test/hello.cpp 
  #include 
  using std::cout;

  int main() {
  cout << "Hello World!\n";
  return 0;
  }

  $ mips-linux-gnu-g++ -o hello hello.cpp && ./hello
  qemu: uncaught target signal 11 (Segmentation fault) - core dumped

  Note that 64-bit MIPS and little endian 32-bit MIPS qemu work fine.
  The problem is limited to big endian 32-bit MIPS.

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

[PULL 0/1] target/openrisc patch queue

The following changes since commit 28b58f19d269633b3d14b6aebf1e92b3cd3ab56e:

  ui/gtk: Get display refresh rate with GDK version 3.22 or later (2020-01-16 
14:03:45 +)

are available in the Git repository at:

  https://github.com/rth7680/qemu.git tags/pull-or1k-20200116

for you to fetch changes up to 97a254b3f03a184136e381c6d9fd80475e1795ac:

  target/openrisc: Fix FPCSR mask to allow setting DZF (2020-01-16 14:50:43 
-1000)


Fix FPSCR masking


Stafford Horne (1):
  target/openrisc: Fix FPCSR mask to allow setting DZF

 target/openrisc/fpu_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

[PULL 1/1] target/openrisc: Fix FPCSR mask to allow setting DZF

From: Stafford Horne 

The mask used when setting FPCSR allows setting bits 10 to 1.  However,
OpenRISC has flags and config bits in 11 to 1, 11 being Divide by Zero
Flag (DZF).  This seems like an off-by-one bug.

This was found when testing the GLIBC test suite which has test cases to
set and clear all bits.

Signed-off-by: Stafford Horne 
Message-Id: <20200110212843.27335-1-sho...@gmail.com>
Signed-off-by: Richard Henderson 
---
 target/openrisc/fpu_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/openrisc/fpu_helper.c b/target/openrisc/fpu_helper.c
index 59e1413279..6f75ea0505 100644
--- a/target/openrisc/fpu_helper.c
+++ b/target/openrisc/fpu_helper.c
@@ -70,7 +70,7 @@ void cpu_set_fpcsr(CPUOpenRISCState *env, uint32_t val)
 float_round_down
 };
 
-env->fpcsr = val & 0x7ff;
+env->fpcsr = val & 0xfff;
 set_float_rounding_mode(rm_to_sf[extract32(val, 1, 2)], &env->fp_status);
 }
 
-- 
2.20.1

Re: [PATCH] target/openrisc: Fix FPCSR mask to allow setting DZF

On 1/10/20 11:28 AM, Stafford Horne wrote:
> The mask used when setting FPCSR allows setting bits 10 to 1.  However,
> OpenRISC has flags and config bits in 11 to 1, 11 being Divide by Zero
> Flag (DZF).  This seems like an off-by-one bug.
> 
> This was found when testing the GLIBC test suite which has test cases to
> set and clear all bits.
> 
> Signed-off-by: Stafford Horne 
> ---
>  target/openrisc/fpu_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, queued.


r~

RE: [PATCH 093/104] virtiofsd: introduce inode refcount to prevent use-after-free

2020-01-16 Thread misono.tomoh...@fujitsu.com

> > On Thu, Jan 16, 2020 at 09:25:42PM +0900, Misono Tomohiro wrote:
> > > > From: Stefan Hajnoczi 
> > > >
> > > > If thread A is using an inode it must not be deleted by thread B
> > > > when processing a FUSE_FORGET request.
> > > >
> > > > The FUSE protocol itself already has a counter called nlookup that
> > > > is used in FUSE_FORGET messages.  We cannot trust this counter
> > > > since the untrusted client can manipulate it via FUSE_FORGET messages.
> > > >
> > > > Introduce a new refcount to keep inodes alive for the required lifespan.
> > > > lo_inode_put() must be called to release a reference.  FUSE's
> > > > nlookup counter holds exactly one reference so that the inode
> > > > stays alive as long as the client still wants to remember it.
> > > >
> > > > Note that the lo_inode->is_symlink field is moved to avoid
> > > > creating a hole in the struct due to struct field alignment.
> > > >
> > > > Signed-off-by: Stefan Hajnoczi 
> > > > ---
> > > >  tools/virtiofsd/passthrough_ll.c | 168
> > > > ++-
> > > >  1 file changed, 145 insertions(+), 23 deletions(-)
> > > >
> > > > diff --git a/tools/virtiofsd/passthrough_ll.c
> > > > b/tools/virtiofsd/passthrough_ll.c
> > > > index b19c9ee328..8f4ab8351c 100644
> > > > --- a/tools/virtiofsd/passthrough_ll.c
> > > > +++ b/tools/virtiofsd/passthrough_ll.c
> > > > @@ -99,7 +99,13 @@ struct lo_key {
> > > >
> > > >  struct lo_inode {
> > > >  int fd;
> > > > -bool is_symlink;
> > > > +
> > > > +/*
> > > > + * Atomic reference count for this object.  The nlookup field 
> > > > holds a
> > > > + * reference and release it when nlookup reaches 0.
> > > > + */
> > > > +gint refcount;
> > > > +
> > > >  struct lo_key key;
> > > >
> > > >  /*
> > > > @@ -118,6 +124,8 @@ struct lo_inode {
> > > >  fuse_ino_t fuse_ino;
> > > >  pthread_mutex_t plock_mutex;
> > > >  GHashTable *posix_locks; /* protected by
> > > > lo_inode->plock_mutex */
> > > > +
> > > > +bool is_symlink;
> > > >  };
> > > >
> > > >  struct lo_cred {
> > > > @@ -473,6 +481,23 @@ static ssize_t lo_add_inode_mapping(fuse_req_t 
> > > > req, struct lo_inode *inode)
> > > >  return elem - lo_data(req)->ino_map.elems;  }
> > > >
> > > > +static void lo_inode_put(struct lo_data *lo, struct lo_inode
> > > > +**inodep) {
> > > > +struct lo_inode *inode = *inodep;
> > > > +
> > > > +if (!inode) {
> > > > +return;
> > > > +}
> > > > +
> > > > +*inodep = NULL;
> > > > +
> > > > +if (g_atomic_int_dec_and_test(&inode->refcount)) {
> > > > +close(inode->fd);
> > > > +free(inode);
> > > > +}
> > > > +}
> > > > +
> > > > +/* Caller must release refcount using lo_inode_put() */
> > > >  static struct lo_inode *lo_inode(fuse_req_t req, fuse_ino_t ino)
> > > > {
> > > >  struct lo_data *lo = lo_data(req); @@ -480,6 +505,9 @@ static
> > > > struct lo_inode *lo_inode(fuse_req_t req, fuse_ino_t ino)
> > > >
> > > >  pthread_mutex_lock(&lo->mutex);
> > > >  elem = lo_map_get(&lo->ino_map, ino);
> > > > +if (elem) {
> > > > +g_atomic_int_inc(&elem->inode->refcount);
> > > > +}
> > > >  pthread_mutex_unlock(&lo->mutex);
> > > >
> > > >  if (!elem) {
> > > > @@ -489,10 +517,23 @@ static struct lo_inode *lo_inode(fuse_req_t req, 
> > > > fuse_ino_t ino)
> > > >  return elem->inode;
> > > >  }
> > > >
> > > > +/*
> > > > + * TODO Remove this helper and force callers to hold an inode
> > > > +refcount until
> > > > + * they are done with the fd.  This will be done in a later patch
> > > > +to make
> > > > + * review easier.
> > > > + */
> > > >  static int lo_fd(fuse_req_t req, fuse_ino_t ino)  {
> > > >  struct lo_inode *inode = lo_inode(req, ino);
> > > > -return inode ? inode->fd : -1;
> > > > +int fd;
> > > > +
> > > > +if (!inode) {
> > > > +return -1;
> > > > +}
> > > > +
> > > > +fd = inode->fd;
> > > > +lo_inode_put(lo_data(req), &inode);
> > > > +return fd;
> > > >  }
> > > >
> > > >  static void lo_init(void *userdata, struct fuse_conn_info *conn)
> > > > @@ -547,6 +588,10 @@ static void lo_getattr(fuse_req_t req, fuse_ino_t 
> > > > ino,
> > > >  fuse_reply_attr(req, &buf, lo->timeout);  }
> > > >
> > > > +/*
> > > > + * Increments parent->nlookup and caller must release refcount
> > > > +using
> > > > + * lo_inode_put(&parent).
> > > > + */
> > > >  static int lo_parent_and_name(struct lo_data *lo, struct lo_inode 
> > > > *inode,
> > > >char path[PATH_MAX], struct
> > > > lo_inode **parent)  { @@ -584,6 +629,7 @@ retry:
> > > >  p = &lo->root;
> > > >  pthread_mutex_lock(&lo->mutex);
> > > >  p->nlookup++;
> > > > +g_atomic_int_inc(&p->refcount);
> > > >  pthread_mutex_unlock(&lo->mutex);
> > > >  } else {
> > > >  *last = '\0';
> > >
> > > We need lo_ionde_put() in error path, right?:
> > > https://gitlab.com/virtio-fs

[PATCH v2 1/2] target/arm: Return correct IL bit in merge_syn_data_abort

From: Jeff Kubascik 

The IL bit is set for 32-bit instructions, thus passing false
with the is_16bit parameter to syn_data_abort_with_iss() makes
a syn mask that always has the IL bit set.

Pass is_16bit as true to make the initial syn mask have IL=0,
so that the final IL value comes from or'ing template_syn.

Cc: qemu-sta...@nongnu.org
Fixes: aaa1f954d4ca ("target-arm: A64: Create Instruction Syndromes for Data 
Aborts")
Signed-off-by: Jeff Kubascik 
[rth: Extracted this as a self-contained bug fix from a larger patch]
Signed-off-by: Richard Henderson 
---
 target/arm/tlb_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
index 5feb312941..e63f8bda29 100644
--- a/target/arm/tlb_helper.c
+++ b/target/arm/tlb_helper.c
@@ -44,7 +44,7 @@ static inline uint32_t merge_syn_data_abort(uint32_t 
template_syn,
 syn = syn_data_abort_with_iss(same_el,
   0, 0, 0, 0, 0,
   ea, 0, s1ptw, is_write, fsc,
-  false);
+  true);
 /* Merge the runtime syndrome with the template syndrome.  */
 syn |= template_syn;
 }
-- 
2.20.1

[PATCH v2 0/2] target/arm: Fix ISSIs16Bit

Changes in v2:
  - Include the merge_syn_data_abort fix, as a self-contained patch.


r~


Jeff Kubascik (1):
  target/arm: Return correct IL bit in merge_syn_data_abort

Richard Henderson (1):
  target/arm: Set ISSIs16Bit in make_issinfo

 target/arm/tlb_helper.c | 2 +-
 target/arm/translate.c  | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

-- 
2.20.1

[PATCH v2 2/2] target/arm: Set ISSIs16Bit in make_issinfo

During the conversion to decodetree, the setting of
ISSIs16Bit got lost.  This causes the guest os to
incorrectly adjust trapping memory operations.

Cc: qemu-sta...@nongnu.org
Fixes: 46beb58efbb8a2a32 ("target/arm: Convert T16, load (literal)")
Reported-by: Jeff Kubascik 
Signed-off-by: Richard Henderson 
---
 target/arm/translate.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 5185e08641..c25921ef95 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -8556,6 +8556,9 @@ static ISSInfo make_issinfo(DisasContext *s, int rd, bool 
p, bool w)
 /* ISS not valid if writeback */
 if (p && !w) {
 ret = rd;
+if (s->base.pc_next - s->pc_curr == 2) {
+ret |= ISSIs16Bit;
+}
 } else {
 ret = ISSInvalid;
 }
-- 
2.20.1

Re: [PATCH V2] vhost-user-test: fix a memory leak

2020-01-16 Thread Pan Nengyuan




On 1/16/2020 9:29 PM, Thomas Huth wrote:
> On 15/01/2020 10.13, Thomas Huth wrote:
>> On 15/01/2020 04.10, Pan Nengyuan wrote:
>>>
>>> On 1/13/2020 10:32 AM, Pan Nengyuan wrote:

 On 1/12/2020 6:39 PM, Thomas Huth wrote:
>> [...]
> ... and now I had to unqueue the patch again. It is reproducibly causing
> one of the gitlab CI pipelines to fail with a timeout, e.g.:
>
>  https://gitlab.com/huth/qemu/-/jobs/400101552
>
> Not sure what is going on here, though, there is no obvious error
> message in the output... this needs some more investigation... do you
> have a gitlab account and could have a look?
>

 OK, I will register a account and have a look.

>>>
>>> I'm sorry, I build and test with the same params, but I can't reproduce it.
>>> Could you add "V=1 or V=2" params to get more information ?
>>
>> It seems to hang forever in qos-test
>> /arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/announce-self
>> :
>>
>>  https://gitlab.com/huth/qemu/-/jobs/403472594
>>
>> It's completely weird, I also added some fprintf statements:
>>
>>  https://gitlab.com/huth/qemu/commit/8ae76c0cf37cf46d26620dd
>>
>> ... but none of them show up in the output of the test run... so I'm
>> currently completely puzzled what might be going wrong here... Any other
>> ideas what we could try here?
> 
> I tried to add some more fprintfs here and there to see where it hangs,
> but I did not succeed to get any further.
> 
> However, the CI build succeeds with this fix instead:
> 
> diff a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
> --- a/tests/qtest/vhost-user-test.c
> +++ b/tests/qtest/vhost-user-test.c
> @@ -707,9 +707,9 @@ static void test_read_guest_mem(void *obj, void
> *arg, QGuestAllocator *alloc)
>  static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc)
>  {
>  TestServer *s = arg;
> -TestServer *dest = test_server_new("dest");
> -GString *dest_cmdline = g_string_new(qos_get_current_command_line());
> -char *uri = g_strdup_printf("%s%s", "unix:", dest->mig_path);
> +TestServer *dest;
> +GString *dest_cmdline;
> +char *uri;
>  QTestState *to;
>  GSource *source;
>  QDict *rsp;
> @@ -720,6 +720,10 @@ static void test_migrate(void *obj, void *arg,
> QGuestAllocator *alloc)
>  return;
>  }
> 
> +dest = test_server_new("dest");
> +dest_cmdline = g_string_new(qos_get_current_command_line());
> +uri = g_strdup_printf("%s%s", "unix:", dest->mig_path);
> +
>  size = get_log_size(s);
>  g_assert_cmpint(size, ==, (256 * 1024 * 1024) / (VHOST_LOG_PAGE * 8));
> 
> @@ -778,6 +782,7 @@ static void test_migrate(void *obj, void *arg,
> QGuestAllocator *alloc)
>  qtest_quit(to);
>  test_server_free(dest);
>  g_free(uri);
> +g_string_free(dest_cmdline, true);
>  }
> 
> 
> Here's a build with that patch that succeeded:
> 
>  https://gitlab.com/huth/qemu/-/jobs/405357307
> 
> So if you agree with that patch, I think we should simply use that
> version instead, ok?

Ok, I agree with it.

Thanks.

> 
>  Thomas
> 
> .
>

Re: [PATCH v3 0/2] Fix hyperv synic on vhost

2020-01-16 Thread no-reply

Patchew URL: 
https://patchew.org/QEMU/20200116202414.157959-1-dgilb...@redhat.com/



Hi,

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

Message-id: 20200116202414.157959-1-dgilb...@redhat.com
Type: series
Subject: [PATCH v3 0/2] Fix hyperv synic on vhost

=== 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/20200116202414.157959-1-dgilb...@redhat.com -> 
patchew/20200116202414.157959-1-dgilb...@redhat.com
Switched to a new branch 'test'
f7aeff2 vhost: Only align sections for vhost-user
5bb467f vhost: Add names to section rounded warning

=== OUTPUT BEGIN ===
1/2 Checking commit 5bb467f4ac3b (vhost: Add names to section rounded warning)
2/2 Checking commit f7aeff24a99a (vhost: Only align sections for vhost-user)
ERROR: trailing whitespace
#45: FILE: hw/virtio/vhost.c:554:
+if (dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {   $

WARNING: line over 80 characters
#60: FILE: hw/virtio/vhost.c:569:
+trace_vhost_region_add_section_aligned(section->mr->name, mrs_gpa, 
mrs_size,

total: 1 errors, 1 warnings, 43 lines checked

Patch 2/2 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/20200116202414.157959-1-dgilb...@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH] util/cacheinfo: fix crash when compiling with uClibc

On 12/16/19 1:18 AM, Carlos Santos wrote:
> On Thu, Oct 17, 2019 at 8:06 PM Carlos Santos  wrote:
>>
>> On Thu, Oct 17, 2019 at 9:47 AM Peter Maydell  
>> wrote:
>>>
>>> On Thu, 17 Oct 2019 at 13:39,  wrote:

 From: Carlos Santos 

 uClibc defines _SC_LEVEL1_ICACHE_LINESIZE and _SC_LEVEL1_DCACHE_LINESIZE
 but the corresponding sysconf calls returns -1, which is a valid result,
 meaning that the limit is indeterminate.

 Handle this situation using the fallback values instead of crashing due
 to an assertion failure.

 Signed-off-by: Carlos Santos 
 ---
  util/cacheinfo.c | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

 diff --git a/util/cacheinfo.c b/util/cacheinfo.c
 index ea6f3e99bf..d94dc6adc8 100644
 --- a/util/cacheinfo.c
 +++ b/util/cacheinfo.c
 @@ -93,10 +93,16 @@ static void sys_cache_info(int *isize, int *dsize)
  static void sys_cache_info(int *isize, int *dsize)
  {
  # ifdef _SC_LEVEL1_ICACHE_LINESIZE
 -*isize = sysconf(_SC_LEVEL1_ICACHE_LINESIZE);
 +int tmp_isize = (int) sysconf(_SC_LEVEL1_ICACHE_LINESIZE);
>>>
>>> Do we need the cast here ?
>>
>> It's there to remind the reader that a type coercion may occur, since
>> sysconf() returns a long and isize is an int.
>>
 +if (tmp_isize > 0) {
 +*isize = tmp_isize;
 +}
  # endif
  # ifdef _SC_LEVEL1_DCACHE_LINESIZE
 -*dsize = sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
 +int tmp_dsize = (int) sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
 +if (tmp_dsize > 0) {
 +*dsize = tmp_dsize;
 +}
  # endif
  }
  #endif /* sys_cache_info */
 --
>>>
>>> thanks
>>> -- PMM
>>
>> --
>> Carlos Santos
>> Senior Software Maintenance Engineer
>> Red Hat
>> casan...@redhat.comT: +55-11-3534-6186
> 
> Hi,
> 
> Any chance to have this merged for Christmas? :-)

No, but it's queued now.  ;-)


r~

[Bug 1860053] [NEW] Possible lack of precision when calling clock_gettime via vDSO on user mode ppc64le

2020-01-16 Thread Patrick Meiring

Public bug reported:

Occurs on QEMU v4.2.0 run on docker (via the qemu-user-static:v4.2.0-2
image) on an AMD64 Ubuntu 18.04.3 LTS machine provided by travis-ci.org.

>From golang's https://github.com/golang/go/issues/36592:

It was discovered that golang's time.NewTicker() and time.Sleep()
malfunction when a compiled application was run via QEMU's ppc64le
emulator in user mode.

The methods did not malfunction on actual PowerPC hardware or when the
same golang application was compiled for golang's arm, arm64 or 386
targets and was run via user mode QEMU on the same system.

Curiously, the methods also worked when the program was compiled under
go 1.11, but do malfunction in go 1.12 and 1.13.

It was identified the change in behaviour was most likely attributable to 
golang switching to using vSDO for calling clock_gettime() on PowerPC 64 
architectures in 1.12. I.E:
https://github.com/golang/go/commit/dbd8af74723d2c98cbdcc70f7e2801f69b57ac5b

We therefore suspect there may be a bug in QEMU's user-mode emulation of
ppc64le as relates to vDSO calls to clock_gettime().

The nature of the malfunction of time.NewTicker() and time.Sleep() is
such that sleeps or ticks with a granularity of less than one second do
not appear to be possible (they all revert to 1 second sleeps/ticks).
Could it be that the nanoseconds field of clock_gettime() is getting
lost in the vDSO version but not in the syscall? Or some other issue
calling these methods via vDSO?

Thanks in advance.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Possible lack of precision when calling clock_gettime via vDSO on user
  mode ppc64le

Status in QEMU:
  New

Bug description:
  Occurs on QEMU v4.2.0 run on docker (via the qemu-user-static:v4.2.0-2
  image) on an AMD64 Ubuntu 18.04.3 LTS machine provided by travis-
  ci.org.

  From golang's https://github.com/golang/go/issues/36592:

  It was discovered that golang's time.NewTicker() and time.Sleep()
  malfunction when a compiled application was run via QEMU's ppc64le
  emulator in user mode.

  The methods did not malfunction on actual PowerPC hardware or when the
  same golang application was compiled for golang's arm, arm64 or 386
  targets and was run via user mode QEMU on the same system.

  Curiously, the methods also worked when the program was compiled under
  go 1.11, but do malfunction in go 1.12 and 1.13.

  It was identified the change in behaviour was most likely attributable to 
golang switching to using vSDO for calling clock_gettime() on PowerPC 64 
architectures in 1.12. I.E:
  https://github.com/golang/go/commit/dbd8af74723d2c98cbdcc70f7e2801f69b57ac5b

  We therefore suspect there may be a bug in QEMU's user-mode emulation
  of ppc64le as relates to vDSO calls to clock_gettime().

  The nature of the malfunction of time.NewTicker() and time.Sleep() is
  such that sleeps or ticks with a granularity of less than one second
  do not appear to be possible (they all revert to 1 second
  sleeps/ticks). Could it be that the nanoseconds field of
  clock_gettime() is getting lost in the vDSO version but not in the
  syscall? Or some other issue calling these methods via vDSO?

  Thanks in advance.

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

Re: [PATCH v2 4/4] qcow2: Use BDRV_SECTOR_SIZE instead of the hardcoded value

2020-01-16 Thread Alberto Garcia

On Tue 14 Jan 2020 03:15:48 PM CET, Max Reitz wrote:
>> @@ -219,7 +219,7 @@ static int l2_load(BlockDriverState *bs, uint64_t offset,
>>   * Writes one sector of the L1 table to the disk (can't update single 
>> entries
>>   * and we really don't want bdrv_pread to perform a read-modify-write)
>>   */
>> -#define L1_ENTRIES_PER_SECTOR (512 / 8)
>> +#define L1_ENTRIES_PER_SECTOR (BDRV_SECTOR_SIZE / 8)
>>  int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index)
>
> Here it’s because the comment is wrong: “Can’t update single entries” –
> yes, we can.  We’d just have to do a bdrv_pwrite() to a single entry.

What's the point of qcow2_write_l1_entry() then?

>> @@ -3836,7 +3837,7 @@ qcow2_co_copy_range_from(BlockDriverState *bs,
>>  case QCOW2_CLUSTER_NORMAL:
>>  child = s->data_file;
>>  copy_offset += offset_into_cluster(s, src_offset);
>> -if ((copy_offset & 511) != 0) {
>> +if (!QEMU_IS_ALIGNED(copy_offset, BDRV_SECTOR_SIZE)) {
>
> Hm.  I don’t get this one.

Checking the code (e.g. block_copy_do_copy()) it seems that the whole
chunk must be cluster aligned so I don't get this one either.

Berto

[PATCH 1/4] target/arm: Fix PAuth sbox functions

From: Vincent Dehors 

In the PAC computation, sbox was applied over wrong bits.
As this is a 4-bit sbox, bit index should be incremented by 4 instead of 16.

Test vector from QARMA paper (https://eprint.iacr.org/2016/444.pdf) was
used to verify one computation of the pauth_computepac() function which
uses sbox2.

Launchpad: https://bugs.launchpad.net/bugs/1859713
Reviewed-by: Richard Henderson 
Signed-off-by: Vincent DEHORS 
Signed-off-by: Adrien GRASSEIN 
---
 target/arm/pauth_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/pauth_helper.c b/target/arm/pauth_helper.c
index d3194f2043..0a5f41e10c 100644
--- a/target/arm/pauth_helper.c
+++ b/target/arm/pauth_helper.c
@@ -89,7 +89,7 @@ static uint64_t pac_sub(uint64_t i)
 uint64_t o = 0;
 int b;
 
-for (b = 0; b < 64; b += 16) {
+for (b = 0; b < 64; b += 4) {
 o |= (uint64_t)sub[(i >> b) & 0xf] << b;
 }
 return o;
@@ -104,7 +104,7 @@ static uint64_t pac_inv_sub(uint64_t i)
 uint64_t o = 0;
 int b;
 
-for (b = 0; b < 64; b += 16) {
+for (b = 0; b < 64; b += 4) {
 o |= (uint64_t)inv_sub[(i >> b) & 0xf] << b;
 }
 return o;
-- 
2.20.1

Re: [PATCH 0/4] migration: Replace gemu_log with qemu_log

2020-01-16 Thread Josh Kunz

On Mon, Jan 13, 2020 at 7:06 PM Warner Losh  wrote:

> The bsd-user that is in tree doesn't work. I've been trying to catch up to
> qemu head of tree, but I'm only up to 3.2... chances are the bsd-user
> changes will not change the state of things...
>

Ah good to know. Would you all prefer I don't modify it at all, to try and
keep the code "pristine"? Or is it OK to keep the patch as is?

Re: [PATCH 0/4] migration: Replace gemu_log with qemu_log

2020-01-16 Thread Josh Kunz

On Tue, Jan 14, 2020 at 3:02 AM Alex Bennée  wrote:

>
> Josh Kunz  writes:
>
> 
> >
> > Not tested:
> >   * Build/logging with bsd-user. I do not have easy access to a BSD
> > system.
>
> If you have the time we have vm-build-netbsd:
>
>   make vm-build-netbsd EXTRA_CONFIGURE_OPTS="--disable-system"
>
> Which will create a NetBSD image for you and run the build through it.
> Other images are available ;-)
>

This works, but it looks like it only runs the tests on BSD, even with
`configure --enable-bsd-user` first. I don't see the bsd-user binaries
being produced in the output of this command.

[PATCH 2/4] tests/tcg/aarch64: Fix compilation parameters for pauth-%

Hiding the required architecture within asm() is not correct.
Add it to the cflags of the (cross-) compiler.

Signed-off-by: Richard Henderson 
---
 tests/tcg/aarch64/pauth-1.c   | 2 --
 tests/tcg/aarch64/pauth-2.c   | 2 --
 tests/tcg/aarch64/Makefile.target | 1 +
 3 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/tests/tcg/aarch64/pauth-1.c b/tests/tcg/aarch64/pauth-1.c
index a3c1443cd0..ea0984ea82 100644
--- a/tests/tcg/aarch64/pauth-1.c
+++ b/tests/tcg/aarch64/pauth-1.c
@@ -2,8 +2,6 @@
 #include 
 #include 
 
-asm(".arch armv8.4-a");
-
 #ifndef PR_PAC_RESET_KEYS
 #define PR_PAC_RESET_KEYS  54
 #define PR_PAC_APDAKEY (1 << 2)
diff --git a/tests/tcg/aarch64/pauth-2.c b/tests/tcg/aarch64/pauth-2.c
index 2fe030ba3d..9bba0beb63 100644
--- a/tests/tcg/aarch64/pauth-2.c
+++ b/tests/tcg/aarch64/pauth-2.c
@@ -1,8 +1,6 @@
 #include 
 #include 
 
-asm(".arch armv8.4-a");
-
 void do_test(uint64_t value)
 {
 uint64_t salt1, salt2;
diff --git a/tests/tcg/aarch64/Makefile.target 
b/tests/tcg/aarch64/Makefile.target
index df3fe8032c..374c8d6830 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -20,6 +20,7 @@ run-fcvt: fcvt
 # Pauth Tests
 AARCH64_TESTS += pauth-1 pauth-2
 run-pauth-%: QEMU_OPTS += -cpu max
+pauth-%: CFLAGS += -march=armv8.3-a
 
 # Semihosting smoke test for linux-user
 AARCH64_TESTS += semihosting
-- 
2.20.1

[PATCH 4/4] tests/tcg/aarch64: Add pauth-4

Perform the set of operations and test described in LP 1859713.

Suggested-by: Adrien GRASSEIN 
Signed-off-by: Richard Henderson 
---
 tests/tcg/aarch64/pauth-4.c   | 25 +
 tests/tcg/aarch64/Makefile.target |  2 +-
 2 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/aarch64/pauth-4.c

diff --git a/tests/tcg/aarch64/pauth-4.c b/tests/tcg/aarch64/pauth-4.c
new file mode 100644
index 00..4b22e6e282
--- /dev/null
+++ b/tests/tcg/aarch64/pauth-4.c
@@ -0,0 +1,25 @@
+#include 
+#include 
+
+int main()
+{
+  uintptr_t x, y;
+
+  asm("mov %0, lr\n\t"
+  "pacia %0, sp\n\t"   /* sigill if pauth not supported */
+  "eor %0, %0, #4\n\t" /* corrupt single bit */
+  "mov %1, %0\n\t"
+  "autia %1, sp\n\t"   /* validate corrupted pointer */
+  "xpaci %0\n\t"   /* strip pac from corrupted pointer */
+  : "=r"(x), "=r"(y));
+
+  /*
+   * Once stripped, the corrupted pointer is of the form 0x...wxyz.
+   * We expect the autia to indicate failure, producing a pointer of the
+   * form 0x000ewxyz.  Use xpaci and != for the test, rather than
+   * extracting explicit bits from the top, because the location of the
+   * error code "e" depends on the configuration of virtual memory.
+   */
+  assert(x != y);
+  return 0;
+}
diff --git a/tests/tcg/aarch64/Makefile.target 
b/tests/tcg/aarch64/Makefile.target
index 374c8d6830..efa67cf1e9 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -18,7 +18,7 @@ run-fcvt: fcvt
$(call diff-out,$<,$(AARCH64_SRC)/fcvt.ref)
 
 # Pauth Tests
-AARCH64_TESTS += pauth-1 pauth-2
+AARCH64_TESTS += pauth-1 pauth-2 pauth-4
 run-pauth-%: QEMU_OPTS += -cpu max
 pauth-%: CFLAGS += -march=armv8.3-a
 
-- 
2.20.1

[PATCH 3/4] tests/tcg/aarch64: Add pauth-3

This is the test vector from the QARMA paper, run through PACGA.

Suggested-by: Vincent Dehors 
Signed-off-by: Richard Henderson 
---
 tests/tcg/aarch64/system/pauth-3.c| 40 +++
 tests/tcg/aarch64/Makefile.softmmu-target |  5 ++-
 2 files changed, 44 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/aarch64/system/pauth-3.c

diff --git a/tests/tcg/aarch64/system/pauth-3.c 
b/tests/tcg/aarch64/system/pauth-3.c
new file mode 100644
index 00..42eff4d5ea
--- /dev/null
+++ b/tests/tcg/aarch64/system/pauth-3.c
@@ -0,0 +1,40 @@
+#include 
+#include 
+
+int main()
+{
+/*
+ * Test vector from QARMA paper (https://eprint.iacr.org/2016/444.pdf)
+ * to verify one computation of the pauth_computepac() function,
+ * which uses sbox2.
+ *
+ * Use PACGA, because it returns the most bits from ComputePAC.
+ * We still only get the most significant 32-bits of the result.
+ */
+
+static const uint64_t d[5] = {
+0xfb623599da6e8127ull,
+0x477d469dec0b8762ull,
+0x84be85ce9804e94bull,
+0xec2802d4e0a488e9ull,
+0xc003b93999b33765ull & 0xull
+};
+uint64_t r;
+
+asm("msr apgakeyhi_el1, %[w0]\n\t"
+"msr apgakeylo_el1, %[k0]\n\t"
+"pacga %[r], %[P], %[T]"
+: [r] "=r"(r)
+: [P] "r" (d[0]),
+  [T] "r" (d[1]),
+  [w0] "r" (d[2]),
+  [k0] "r" (d[3]));
+
+if (r == d[4]) {
+ml_printf("OK\n");
+return 0;
+} else {
+ml_printf("FAIL: %lx != %lx\n", r, d[4]);
+return 1;
+}
+}
diff --git a/tests/tcg/aarch64/Makefile.softmmu-target 
b/tests/tcg/aarch64/Makefile.softmmu-target
index 7b4eede3f0..f6b5121f5c 100644
--- a/tests/tcg/aarch64/Makefile.softmmu-target
+++ b/tests/tcg/aarch64/Makefile.softmmu-target
@@ -61,4 +61,7 @@ run-memory-replay: memory-replay run-memory-record
  $(QEMU_OPTS) memory, \
  "$< on $(TARGET_NAME)")
 
-EXTRA_TESTS+=memory-record memory-replay
+run-pauth-3: pauth-3
+pauth-3: CFLAGS += -march=armv8.3-a
+
+EXTRA_TESTS+=memory-record memory-replay pauth-3
-- 
2.20.1

[PATCH 0/4] target/arm: Fix ComputePAC (LP 1859713)

Thanks to Vincent and Adrien for both reporting the bug and
providing the solution.  I've converted their manual testing
into executable tests.


r~


Richard Henderson (3):
  tests/tcg/aarch64: Fix compilation parameters for pauth-%
  tests/tcg/aarch64: Add pauth-3
  tests/tcg/aarch64: Add pauth-4

Vincent Dehors (1):
  target/arm: Fix PAuth sbox functions

 target/arm/pauth_helper.c |  4 +--
 tests/tcg/aarch64/pauth-1.c   |  2 --
 tests/tcg/aarch64/pauth-2.c   |  2 --
 tests/tcg/aarch64/pauth-4.c   | 25 ++
 tests/tcg/aarch64/system/pauth-3.c| 40 +++
 tests/tcg/aarch64/Makefile.softmmu-target |  5 ++-
 tests/tcg/aarch64/Makefile.target |  3 +-
 7 files changed, 73 insertions(+), 8 deletions(-)
 create mode 100644 tests/tcg/aarch64/pauth-4.c
 create mode 100644 tests/tcg/aarch64/system/pauth-3.c

-- 
2.20.1

[PATCH 04/12] linux-user: Add support for FS_IOC_FSXATTR ioctls

From: Aleksandar Markovic 

Both FS_IOC_FSGETXATTR and FS_IOC_FSSETXATTR accept a pointer to
the structure

struct fsxattr {
__u32 fsx_xflags; /* xflags field value (get/set) */
__u32 fsx_extsize;/* extsize field value (get/set)*/
__u32 fsx_nextents;   /* nextents field value (get) */
__u32 fsx_projid; /* project identifier (get/set) */
__u32 fsx_cowextsize; /* CoW extsize field value (get/set)*/
unsigned char fsx_pad[8];
};

as their third argument.

These ioctls were relatively recently introduced, so the "#ifdef"
guards are used in this implementation.

Signed-off-by: Aleksandar Markovic 
---
 linux-user/ioctls.h   | 7 +++
 linux-user/syscall_defs.h | 6 ++
 2 files changed, 13 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 3affd88..e1b89a7 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -144,6 +144,13 @@
  IOCTL(FS_IOC32_SETFLAGS, IOC_W, MK_PTR(TYPE_INT))
  IOCTL(FS_IOC32_GETVERSION, IOC_R, MK_PTR(TYPE_INT))
  IOCTL(FS_IOC32_SETVERSION, IOC_W, MK_PTR(TYPE_INT))
+#ifdef FS_IOC_FSGETXATTR
+ IOCTL(FS_IOC_FSGETXATTR, IOC_W, MK_PTR(MK_STRUCT(STRUCT_fsxattr)))
+#endif
+#ifdef FS_IOC_FSSETXATTR
+ IOCTL(FS_IOC_FSSETXATTR, IOC_W, MK_PTR(MK_STRUCT(STRUCT_fsxattr)))
+#endif
+
 
 #ifdef CONFIG_USBFS
   /* USB ioctls */
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index a73cc3d..e1663b6 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -924,6 +924,12 @@ struct target_pollfd {
 #define TARGET_FS_IOC32_SETFLAGS TARGET_IOW('f', 2, int)
 #define TARGET_FS_IOC32_GETVERSION TARGET_IOR('v', 1, int)
 #define TARGET_FS_IOC32_SETVERSION TARGET_IOW('v', 2, int)
+#ifdef FS_IOC_FSGETXATTR
+#define TARGET_FS_IOC_FSGETXATTR TARGET_IOR('X', 31, struct fsxattr)
+#endif
+#ifdef FS_IOC_FSSETXATTR
+#define TARGET_FS_IOC_FSSETXATTR TARGET_IOR('X', 32, struct fsxattr)
+#endif
 
 /* usb ioctls */
 #define TARGET_USBDEVFS_CONTROL TARGET_IOWRU('U', 0)
-- 
2.7.4

[PATCH 08/12] linux-user: Add support for FDFMT ioctls

From: Aleksandar Markovic 

FDFMTBEG, FDFMTTRK, and FDFMTEND ioctls provide means for controlling
formatting of a floppy drive.

FDFMTTRK's third agrument is a pointer to the structure:

struct format_descr {
unsigned int device,head,track;
};

defined in Linux kernel header .

Since all fields of the structure are of type 'unsigned int', there is
no need to define "target_format_descr".

FDFMTBEG and FDFMTEND ioctls do not use the third argument.

Reviewed-by: Laurent Vivier 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/ioctls.h| 3 +++
 linux-user/syscall_defs.h  | 3 +++
 linux-user/syscall_types.h | 5 +
 3 files changed, 11 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 9e3ca90..e754a6b 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -115,6 +115,9 @@
  IOCTL(FDMSGON, 0, TYPE_NULL)
  IOCTL(FDMSGOFF, 0, TYPE_NULL)
  IOCTL(FDSETEMSGTRESH, 0, TYPE_NULL)
+ IOCTL(FDFMTBEG, 0, TYPE_NULL)
+ IOCTL(FDFMTTRK, IOC_W, MK_PTR(MK_STRUCT(STRUCT_format_descr)))
+ IOCTL(FDFMTEND, 0, TYPE_NULL)
  IOCTL(FDFLUSH, 0, TYPE_NULL)
  IOCTL(FDSETMAXERRS, IOC_W, MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
  IOCTL(FDGETMAXERRS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index e317115..9fbf04a 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -899,6 +899,9 @@ struct target_pollfd {
 
 #define TARGET_FDMSGONTARGET_IO(2, 0x45)
 #define TARGET_FDMSGOFF   TARGET_IO(2, 0x46)
+#define TARGET_FDFMTBEG   TARGET_IO(2, 0x47)
+#define TARGET_FDFMTTRK  TARGET_IOW(2, 0x48, struct format_descr)
+#define TARGET_FDFMTEND   TARGET_IO(2, 0x49)
 #define TARGET_FDSETEMSGTRESH TARGET_IO(2, 0x4a)
 #define TARGET_FDFLUSHTARGET_IO(2, 0x4b)
 #define TARGET_FDSETMAXERRS  TARGET_IOW(2, 0x4c, struct floppy_max_errors)
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index 434ce1c..ff60240 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -266,6 +266,11 @@ STRUCT(blkpg_ioctl_arg,
TYPE_INT, /* datalen */
TYPE_PTRVOID) /* data */
 
+STRUCT(format_descr,
+   TYPE_INT, /* device */
+   TYPE_INT, /* head */
+   TYPE_INT) /* track */
+
 STRUCT(floppy_max_errors,
TYPE_INT, /* abort */
TYPE_INT, /* read_track */
-- 
2.7.4

[PATCH 09/12] linux-user: Add support for FDGETFDCSTAT ioctl

From: Aleksandar Markovic 

FDGETFDCSTAT's third agrument is a pointer to the structure:

struct floppy_fdc_state {
int spec1;
int spec2;
int dtr;
unsigned char version;
unsigned char dor;
unsigned long address;
unsigned int rawcmd:2;
unsigned int reset:1;
unsigned int need_configure:1;
unsigned int perp_mode:2;
unsigned int has_fifo:1;
unsigned int driver_version;
unsigned char track[4];
};

defined in Linux kernel header .

Since there is a fields of the structure of type 'unsigned long', there is
a need to define "target_format_descr". Also, five fields rawcmd, reset,
need_configure, perp_mode, and has_fifo are all just bitfields and are
part od a single 'unsigned int' field.

Signed-off-by: Aleksandar Markovic 
---
 linux-user/ioctls.h|  2 ++
 linux-user/syscall_defs.h  | 18 ++
 linux-user/syscall_types.h | 12 
 3 files changed, 32 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index e754a6b..d72cd76 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -122,6 +122,8 @@
  IOCTL(FDSETMAXERRS, IOC_W, MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
  IOCTL(FDGETMAXERRS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
  IOCTL(FDRESET, 0, TYPE_NULL)
+ IOCTL(FDGETFDCSTAT, IOC_R,
+   MK_PTR(MK_STRUCT(STRUCT_target_floppy_fdc_state)))
  IOCTL(FDRAWCMD, 0, TYPE_NULL)
  IOCTL(FDTWADDLE, 0, TYPE_NULL)
  IOCTL(FDEJECT, 0, TYPE_NULL)
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 9fbf04a..46dc565 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -897,6 +897,23 @@ struct target_pollfd {
 
 /* From  */
 
+struct target_floppy_fdc_state {
+int spec1;  /* spec1 value last used */
+int spec2;  /* spec2 value last used */
+int dtr;
+unsigned char version;  /* FDC version code */
+unsigned char dor;
+abi_ulong address;  /* io address */
+unsigned int rawcmd:2;
+unsigned int reset:1;
+unsigned int need_configure:1;
+unsigned int perp_mode:2;
+unsigned int has_fifo:1;
+unsigned int driver_version;/* version code for floppy driver */
+unsigned char track[4];
+};
+
+
 #define TARGET_FDMSGONTARGET_IO(2, 0x45)
 #define TARGET_FDMSGOFF   TARGET_IO(2, 0x46)
 #define TARGET_FDFMTBEG   TARGET_IO(2, 0x47)
@@ -907,6 +924,7 @@ struct target_pollfd {
 #define TARGET_FDSETMAXERRS  TARGET_IOW(2, 0x4c, struct floppy_max_errors)
 #define TARGET_FDGETMAXERRS  TARGET_IOR(2, 0x0e, struct floppy_max_errors)
 #define TARGET_FDRESETTARGET_IO(2, 0x54)
+#define TARGET_FDGETFDCSTAT  TARGET_IOR(2, 0x15, struct 
target_floppy_fdc_state)
 #define TARGET_FDRAWCMD   TARGET_IO(2, 0x58)
 #define TARGET_FDTWADDLE  TARGET_IO(2, 0x59)
 #define TARGET_FDEJECTTARGET_IO(2, 0x5a)
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index ff60240..2b480d0 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -278,6 +278,18 @@ STRUCT(floppy_max_errors,
TYPE_INT, /* recal */
TYPE_INT) /* reporting */
 
+STRUCT(target_floppy_fdc_state,
+   TYPE_INT, /* spec1 */
+   TYPE_INT, /* spec2 */
+   TYPE_INT, /* dtr */
+   TYPE_CHAR, /* version */
+   TYPE_CHAR, /* dor */
+   TYPE_ULONG, /* address */
+   TYPE_INT, /* bit field for rawcmd:2, reset:1, need_configure:1, */
+ /* perp_mode:2, and has_fifo:1 */
+   TYPE_INT, /* driver_version */
+   MK_ARRAY(TYPE_CHAR, 4)) /* track */
+
 #if defined(CONFIG_USBFS)
 /* usb device ioctls */
 STRUCT(usbdevfs_ctrltransfer,
-- 
2.7.4

[PATCH 11/12] linux-user: Add support for KCOV_ ioctls

From: Aleksandar Markovic 

KCOV_ENABLE and KCOV_DISABLE play the role in kernel coverage
tracing. These ioctls do not use the third argument of ioctl()
system call and are straightforward to implement in QEMU.

Reviewed-by: Laurent Vivier 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/ioctls.h   | 5 +
 linux-user/syscall.c  | 3 +++
 linux-user/syscall_defs.h | 4 
 3 files changed, 12 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index d72cd76..39b3825 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -552,3 +552,8 @@
   IOCTL_IGNORE(TIOCSTART)
   IOCTL_IGNORE(TIOCSTOP)
 #endif
+
+#ifdef CONFIG_KCOV
+  IOCTL(KCOV_ENABLE, 0, TYPE_NULL)
+  IOCTL(KCOV_DISABLE, 0, TYPE_NULL)
+#endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 171c0ca..6edcb0d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -73,6 +73,9 @@
 #ifdef CONFIG_SENDFILE
 #include 
 #endif
+#ifdef CONFIG_KCOV
+#include 
+#endif
 
 #define termios host_termios
 #define winsize host_winsize
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 46dc565..c8999ef 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2461,6 +2461,10 @@ struct target_mtpos {
 #define TARGET_MTIOCGETTARGET_IOR('m', 2, struct target_mtget)
 #define TARGET_MTIOCPOSTARGET_IOR('m', 3, struct target_mtpos)
 
+/* kcov ioctls */
+#define TARGET_KCOV_ENABLE TARGET_IO('c', 100)
+#define TARGET_KCOV_DISABLETARGET_IO('c', 101)
+
 struct target_sysinfo {
 abi_long uptime;/* Seconds since boot */
 abi_ulong loads[3]; /* 1, 5, and 15 minute load averages */
-- 
2.7.4

[PATCH 10/12] configure: Detect kcov support and introduce CONFIG_KCOV

From: Aleksandar Markovic 

kcov is kernel code coverage tracing tool. It requires kernel 4.4+
compiled with certain kernel options.

This patch checks if kcov header "sys/kcov.h" is present on build
machine, and stores the result in variable CONFIG_KCOV, meant to
be used in linux-user code related to the support for three ioctls
that were introduced at the same time as the mentioned header
(their definition was a part of the first version of that header).

Signed-off-by: Aleksandar Markovic 
---
 configure | 9 +
 1 file changed, 9 insertions(+)

diff --git a/configure b/configure
index 940bf9e..57e6eba 100755
--- a/configure
+++ b/configure
@@ -4752,6 +4752,12 @@ if compile_prog "" "" ; then
   syncfs=yes
 fi
 
+# check for kcov support (kernel must be 4.4+, compiled with certain options)
+kcov=no
+if check_include sys/kcov.h ; then
+kcov=yes
+fi
+
 # Check we have a new enough version of sphinx-build
 has_sphinx_build() {
 # This is a bit awkward but works: create a trivial document and
@@ -6874,6 +6880,9 @@ fi
 if test "$syncfs" = "yes" ; then
   echo "CONFIG_SYNCFS=y" >> $config_host_mak
 fi
+if test "$kcov" = "yes" ; then
+  echo "CONFIG_KCOV=y" >> $config_host_mak
+fi
 if test "$inotify" = "yes" ; then
   echo "CONFIG_INOTIFY=y" >> $config_host_mak
 fi
-- 
2.7.4

[PATCH 07/12] linux-user: Add support for FD ioctls

From: Aleksandar Markovic 

FDSETEMSGTRESH, FDSETMAXERRS, and FDGETMAXERRS ioctls are commands
for controlling error reporting of a floppy drive.

FDSETEMSGTRESH's third agrument is a pointer to the structure:

struct floppy_max_errors {
unsigned int
  abort,  /* number of errors to be reached before aborting */
  read_track, /* maximal number of errors permitted to read an
   * entire track at once */
  reset,  /* maximal number of errors before a reset is tried */
  recal,  /* maximal number of errors before a recalibrate is
   * tried */
  /*
   * Threshold for reporting FDC errors to the console.
   * Setting this to zero may flood your screen when using
   * ultra cheap floppies ;-)
   */
  reporting;
};

defined in Linux kernel header .

Since all fields of the structure are of type 'unsigned int', there is
no need to define "target_floppy_max_errors".

FDSETMAXERRS and FDGETMAXERRS ioctls do not use the third argument.

Reviewed-by: Laurent Vivier 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/ioctls.h| 3 +++
 linux-user/syscall_defs.h  | 3 +++
 linux-user/syscall_types.h | 7 +++
 3 files changed, 13 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 66f8c4e..9e3ca90 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -114,7 +114,10 @@
 
  IOCTL(FDMSGON, 0, TYPE_NULL)
  IOCTL(FDMSGOFF, 0, TYPE_NULL)
+ IOCTL(FDSETEMSGTRESH, 0, TYPE_NULL)
  IOCTL(FDFLUSH, 0, TYPE_NULL)
+ IOCTL(FDSETMAXERRS, IOC_W, MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
+ IOCTL(FDGETMAXERRS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
  IOCTL(FDRESET, 0, TYPE_NULL)
  IOCTL(FDRAWCMD, 0, TYPE_NULL)
  IOCTL(FDTWADDLE, 0, TYPE_NULL)
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index d4d39de..e317115 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -899,7 +899,10 @@ struct target_pollfd {
 
 #define TARGET_FDMSGONTARGET_IO(2, 0x45)
 #define TARGET_FDMSGOFF   TARGET_IO(2, 0x46)
+#define TARGET_FDSETEMSGTRESH TARGET_IO(2, 0x4a)
 #define TARGET_FDFLUSHTARGET_IO(2, 0x4b)
+#define TARGET_FDSETMAXERRS  TARGET_IOW(2, 0x4c, struct floppy_max_errors)
+#define TARGET_FDGETMAXERRS  TARGET_IOR(2, 0x0e, struct floppy_max_errors)
 #define TARGET_FDRESETTARGET_IO(2, 0x54)
 #define TARGET_FDRAWCMD   TARGET_IO(2, 0x58)
 #define TARGET_FDTWADDLE  TARGET_IO(2, 0x59)
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index ca9429e..434ce1c 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -266,6 +266,13 @@ STRUCT(blkpg_ioctl_arg,
TYPE_INT, /* datalen */
TYPE_PTRVOID) /* data */
 
+STRUCT(floppy_max_errors,
+   TYPE_INT, /* abort */
+   TYPE_INT, /* read_track */
+   TYPE_INT, /* reset */
+   TYPE_INT, /* recal */
+   TYPE_INT) /* reporting */
+
 #if defined(CONFIG_USBFS)
 /* usb device ioctls */
 STRUCT(usbdevfs_ctrltransfer,
-- 
2.7.4

[PATCH 01/12] linux-user: Add support for FS_IOC_VERSION ioctls

From: Aleksandar Markovic 

A very specific thing for these two ioctls is that their code
implies that their third argument is of type 'long', but the
kernel uses that argument as if it is of type 'int'. This anomaly
is recognized also in commit 6080723 (linux-user: Implement
FS_IOC_GETFLAGS and FS_IOC_SETFLAGS ioctls).

Reviewed-by: Laurent Vivier 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/ioctls.h   | 2 ++
 linux-user/syscall_defs.h | 8 +---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index c6b9d6a..c44f42e 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -138,6 +138,8 @@
 
  IOCTL(FS_IOC_GETFLAGS, IOC_R, MK_PTR(TYPE_INT))
  IOCTL(FS_IOC_SETFLAGS, IOC_W, MK_PTR(TYPE_INT))
+ IOCTL(FS_IOC_GETVERSION, IOC_R, MK_PTR(TYPE_INT))
+ IOCTL(FS_IOC_SETVERSION, IOC_W, MK_PTR(TYPE_INT))
 
 #ifdef CONFIG_USBFS
   /* USB ioctls */
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 98c2119..f68a8b6 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -911,12 +911,14 @@ struct target_pollfd {
 #define TARGET_FICLONETARGET_IOW(0x94, 9, int)
 #define TARGET_FICLONERANGE TARGET_IOW(0x94, 13, struct file_clone_range)
 
-/* Note that the ioctl numbers claim type "long" but the actual type
- * used by the kernel is "int".
+/*
+ * Note that the ioctl numbers for FS_IOC_
+ * claim type "long" but the actual type used by the kernel is "int".
  */
 #define TARGET_FS_IOC_GETFLAGS TARGET_IOR('f', 1, abi_long)
 #define TARGET_FS_IOC_SETFLAGS TARGET_IOW('f', 2, abi_long)
-
+#define TARGET_FS_IOC_GETVERSION TARGET_IOR('v', 1, abi_long)
+#define TARGET_FS_IOC_SETVERSION TARGET_IOW('v', 2, abi_long)
 #define TARGET_FS_IOC_FIEMAP TARGET_IOWR('f',11,struct fiemap)
 
 /* usb ioctls */
-- 
2.7.4

[PATCH 06/12] linux-user: Add support for FIFREEZE and FITHAW ioctls

From: Aleksandar Markovic 

Both FIFREEZE and FITHAW ioctls accept an integer as their third
argument.

All ioctls in this group (FI* ioctl) are guarded with "#ifdef", so the
guards are used in this implementation too for consistency (however,
many of ioctls in FI* group became old enough that their #ifdef guards
could be removed, bit this is out of the scope of this patch).

Reviewed-by: Laurent Vivier 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/ioctls.h   | 6 ++
 linux-user/syscall_defs.h | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index e4f0a04..66f8c4e 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -123,6 +123,12 @@
 #ifdef FIBMAP
  IOCTL(FIBMAP, IOC_W | IOC_R, MK_PTR(TYPE_LONG))
 #endif
+#ifdef FIFREEZE
+ IOCTL(FIFREEZE, IOC_W | IOC_R, TYPE_INT)
+#endif
+#ifdef FITHAW
+ IOCTL(FITHAW, IOC_W | IOC_R, TYPE_INT)
+#endif
 #ifdef FITRIM
  IOCTL(FITRIM, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_fstrim_range)))
 #endif
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 97ab3e1..d4d39de 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -908,6 +908,8 @@ struct target_pollfd {
 #define TARGET_FIBMAP TARGET_IO(0x00,1)  /* bmap access */
 #define TARGET_FIGETBSZ   TARGET_IO(0x00,2)  /* get the block size used for 
bmap */
 
+#define TARGET_FIFREEZE   TARGET_IOWR('X', 119, int)/* Freeze */
+#define TARGET_FITHAW TARGET_IOWR('X', 120, int)/* Thaw */
 #define TARGET_FITRIM TARGET_IOWR('X', 121, struct fstrim_range)
 #define TARGET_FICLONETARGET_IOW(0x94, 9, int)
 #define TARGET_FICLONERANGE TARGET_IOW(0x94, 13, struct file_clone_range)
-- 
2.7.4

[PATCH 05/12] linux-user: Add support for FITRIM ioctl

From: Aleksandar Markovic 

FITRIM ioctl accepts a pointer to the structure

struct fstrim_range {
__u64 start;
__u64 len;
__u64 minlen;
};

as its third argument.

All ioctls in this group (FI* ioctl) are guarded with "#ifdef", so the
guards are used in this implementation too for consistency (however,
many of ioctls in FI* group became old enough that their #ifdef guards
could be removed, bit this is out of the scope of this patch).

Signed-off-by: Aleksandar Markovic 
---
 linux-user/ioctls.h| 3 +++
 linux-user/syscall_defs.h  | 1 +
 linux-user/syscall_types.h | 5 +
 3 files changed, 9 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index e1b89a7..e4f0a04 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -123,6 +123,9 @@
 #ifdef FIBMAP
  IOCTL(FIBMAP, IOC_W | IOC_R, MK_PTR(TYPE_LONG))
 #endif
+#ifdef FITRIM
+ IOCTL(FITRIM, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_fstrim_range)))
+#endif
 #ifdef FICLONE
  IOCTL(FICLONE, IOC_W, TYPE_INT)
  IOCTL(FICLONERANGE, IOC_W, MK_PTR(MK_STRUCT(STRUCT_file_clone_range)))
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index e1663b6..97ab3e1 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -908,6 +908,7 @@ struct target_pollfd {
 #define TARGET_FIBMAP TARGET_IO(0x00,1)  /* bmap access */
 #define TARGET_FIGETBSZ   TARGET_IO(0x00,2)  /* get the block size used for 
bmap */
 
+#define TARGET_FITRIM TARGET_IOWR('X', 121, struct fstrim_range)
 #define TARGET_FICLONETARGET_IOW(0x94, 9, int)
 #define TARGET_FICLONERANGE TARGET_IOW(0x94, 13, struct file_clone_range)
 
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index 4e36983..ca9429e 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -226,6 +226,11 @@ STRUCT(dm_target_versions,
 STRUCT(dm_target_msg,
TYPE_ULONGLONG) /* sector */
 
+STRUCT(fstrim_range,
+   TYPE_LONGLONG, /* start */
+   TYPE_LONGLONG, /* len */
+   TYPE_LONGLONG) /* minlen */
+
 STRUCT(file_clone_range,
TYPE_LONGLONG, /* src_fd */
TYPE_ULONGLONG, /* src_offset */
-- 
2.7.4

[PATCH 00/12] linux-user: Add support for fs, fd,and kcov ioctls

From: Aleksandar Markovic 

This series is a spin-off of v5 of earlier series "linux-user: Misc
patches for 5.0", that became too large to manage. I will submit the
rest of that large series separately.

The only difference between patches in this series and in the former
series is that all Laurent's comments and concerns were addressed.

The summary of the patches is as follows:

Patches 1-6: Adding support for some filesystem-related ioctls
Patches 7-9: Adding support for some floppy-drive-related ioctls
Patches 10-12: Adding support for kcov-related ioctls

Aleksandar Markovic (12):
  linux-user: Add support for FS_IOC_VERSION ioctls
  linux-user: Add support for FS_IOC32_FLAGS ioctls
  linux-user: Add support for FS_IOC32_VERSION ioctls
  linux-user: Add support for FS_IOC_FSXATTR ioctls
  linux-user: Add support for FITRIM ioctl
  linux-user: Add support for FIFREEZE and FITHAW ioctls
  linux-user: Add support for FD
ioctls
  linux-user: Add support for FDFMT ioctls
  linux-user: Add support for FDGETFDCSTAT ioctl
  configure: Detect kcov support and introduce CONFIG_KCOV
  linux-user: Add support for KCOV_ ioctls
  linux-user: Add support for KCOV_INIT_TRACE ioctl

 configure  |  9 +
 linux-user/ioctls.h| 36 +
 linux-user/syscall.c   |  3 +++
 linux-user/syscall_defs.h  | 50 +++---
 linux-user/syscall_types.h | 29 +++
 5 files changed, 124 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH 02/12] linux-user: Add support for FS_IOC32_FLAGS ioctls

From: Aleksandar Markovic 

These FS_IOC32_FLAGS ioctls are identical to
FS_IOC_FLAGS ioctls, but without the anomaly of their
number defined as if their third argument is of type long, while
it is treated internally in kernel as is of type int.

Reviewed-by: Laurent Vivier 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/ioctls.h   | 2 ++
 linux-user/syscall_defs.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index c44f42e..4fd6939 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -140,6 +140,8 @@
  IOCTL(FS_IOC_SETFLAGS, IOC_W, MK_PTR(TYPE_INT))
  IOCTL(FS_IOC_GETVERSION, IOC_R, MK_PTR(TYPE_INT))
  IOCTL(FS_IOC_SETVERSION, IOC_W, MK_PTR(TYPE_INT))
+ IOCTL(FS_IOC32_GETFLAGS, IOC_R, MK_PTR(TYPE_INT))
+ IOCTL(FS_IOC32_SETFLAGS, IOC_W, MK_PTR(TYPE_INT))
 
 #ifdef CONFIG_USBFS
   /* USB ioctls */
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index f68a8b6..964b2b4 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -920,6 +920,8 @@ struct target_pollfd {
 #define TARGET_FS_IOC_GETVERSION TARGET_IOR('v', 1, abi_long)
 #define TARGET_FS_IOC_SETVERSION TARGET_IOW('v', 2, abi_long)
 #define TARGET_FS_IOC_FIEMAP TARGET_IOWR('f',11,struct fiemap)
+#define TARGET_FS_IOC32_GETFLAGS TARGET_IOR('f', 1, int)
+#define TARGET_FS_IOC32_SETFLAGS TARGET_IOW('f', 2, int)
 
 /* usb ioctls */
 #define TARGET_USBDEVFS_CONTROL TARGET_IOWRU('U', 0)
-- 
2.7.4

[PATCH 03/12] linux-user: Add support for FS_IOC32_VERSION ioctls

From: Aleksandar Markovic 

These FS_IOC32_VERSION ioctls are identical to
FS_IOC_VERSION ioctls, but without the anomaly of their
number defined as if their third argument is of type long, while
it is treated internally in kernel as is of type int.

Reviewed-by: Laurent Vivier 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/ioctls.h   | 2 ++
 linux-user/syscall_defs.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 4fd6939..3affd88 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -142,6 +142,8 @@
  IOCTL(FS_IOC_SETVERSION, IOC_W, MK_PTR(TYPE_INT))
  IOCTL(FS_IOC32_GETFLAGS, IOC_R, MK_PTR(TYPE_INT))
  IOCTL(FS_IOC32_SETFLAGS, IOC_W, MK_PTR(TYPE_INT))
+ IOCTL(FS_IOC32_GETVERSION, IOC_R, MK_PTR(TYPE_INT))
+ IOCTL(FS_IOC32_SETVERSION, IOC_W, MK_PTR(TYPE_INT))
 
 #ifdef CONFIG_USBFS
   /* USB ioctls */
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 964b2b4..a73cc3d 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -922,6 +922,8 @@ struct target_pollfd {
 #define TARGET_FS_IOC_FIEMAP TARGET_IOWR('f',11,struct fiemap)
 #define TARGET_FS_IOC32_GETFLAGS TARGET_IOR('f', 1, int)
 #define TARGET_FS_IOC32_SETFLAGS TARGET_IOW('f', 2, int)
+#define TARGET_FS_IOC32_GETVERSION TARGET_IOR('v', 1, int)
+#define TARGET_FS_IOC32_SETVERSION TARGET_IOW('v', 2, int)
 
 /* usb ioctls */
 #define TARGET_USBDEVFS_CONTROL TARGET_IOWRU('U', 0)
-- 
2.7.4

[PATCH 12/12] linux-user: Add support for KCOV_INIT_TRACE ioctl

From: Aleksandar Markovic 

KCOV_INIT_TRACE ioctl plays the role in kernel coverage tracing.
This ioctl's third argument is of type 'unsigned long', and the
implementation in QEMU is straightforward.

Reviewed-by: Laurent Vivier 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/ioctls.h   | 1 +
 linux-user/syscall_defs.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 39b3825..1da71dd 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -556,4 +556,5 @@
 #ifdef CONFIG_KCOV
   IOCTL(KCOV_ENABLE, 0, TYPE_NULL)
   IOCTL(KCOV_DISABLE, 0, TYPE_NULL)
+  IOCTL(KCOV_INIT_TRACE, IOC_R, TYPE_ULONG)
 #endif
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index c8999ef..bf71b3a 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2464,6 +2464,7 @@ struct target_mtpos {
 /* kcov ioctls */
 #define TARGET_KCOV_ENABLE TARGET_IO('c', 100)
 #define TARGET_KCOV_DISABLETARGET_IO('c', 101)
+#define TARGET_KCOV_INIT_TRACE TARGET_IOR('c', 1, abi_ulong)
 
 struct target_sysinfo {
 abi_long uptime;/* Seconds since boot */
-- 
2.7.4

[Bug 1859713] Re: ARM v8.3a pauth not working

Oops again.  The test case has the parts of the key the wrong way around.
I'll submit the pair of patches to the mailing list.

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

Title:
  ARM v8.3a pauth not working

Status in QEMU:
  Confirmed

Bug description:
  Host: Ubuntu 19.10 - x86_64 machine
  QEMU version: 3a63b24a1bbf166e6f455fe43a6bbd8dea413d92 (master)

  ARMV8.3 pauth is not working well.

  With a test code containing two pauth instructions:
  - paciasp that sign LR with A key and sp as context;
  - autiasp that verify the signature.

  Test:
  - Run the program and corrupt LR just before autiasp (ex 0x3e0400660 
instead of 0x3e00400664)

  Expected:
  - autiasp places an invalid pointer in LR

  Result:
  - autiasp successfully auth the pointer and places 0x0400660 in LR.

  Further explanations:
  Adding traces in qemu code shows that "pauth_computepac" is not robust 
enough against truncating.
  With 0x3100400664 as input of pauth_auth, we obtain 
"0x55b1d65b2c138e14" for PAC, "0x30" for bot_bit and "0x38" for top_bit.
  With 0x310040008743ec as input of pauth (with same key), we obtain 
"0x55b1d65b2c138ef4" for PAC, "0x30" for bot_bit and "0x38" for top_bit.
  Values of top_bit and bottom_bit are strictly the same and it should not.

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

[Bug 1859713] Re: ARM v8.3a pauth not working

Ooof. Good catch on the sbox error.

That said, how did you test pauth_computepac?
I still do not get the C5 result above, but 0x99d88f4472f3be39.

The following test case sets up the parameters.

** Patch added: "test case"

https://bugs.launchpad.net/qemu/+bug/1859713/+attachment/5320967/+files/0001-tests-tcg-aarch64-Add-pauth-3.patch

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

Title:
ARM v8.3a pauth not working

Status in QEMU:
Confirmed

Bug description:
Host: Ubuntu 19.10 - x86_64 machine
QEMU version: 3a63b24a1bbf166e6f455fe43a6bbd8dea413d92 (master)

ARMV8.3 pauth is not working well.

With a test code containing two pauth instructions:
- paciasp that sign LR with A key and sp as context;
- autiasp that verify the signature.

Test:
- Run the program and corrupt LR just before autiasp (ex 0x3e0400660
instead of 0x3e00400664)

Expected:
- autiasp places an invalid pointer in LR

Result:
- autiasp successfully auth the pointer and places 0x0400660 in LR.

Further explanations:
Adding traces in qemu code shows that "pauth_computepac" is not robust
enough against truncating.
With 0x3100400664 as input of pauth_auth, we obtain
"0x55b1d65b2c138e14" for PAC, "0x30" for bot_bit and "0x38" for top_bit.
With 0x310040008743ec as input of pauth (with same key), we obtain
"0x55b1d65b2c138ef4" for PAC, "0x30" for bot_bit and "0x38" for top_bit.
Values of top_bit and bottom_bit are strictly the same and it should not.

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

Re: [PATCH v3 02/11] 9pfs: require msize >= 4096

2020-01-16 Thread Christian Schoenebeck

On Donnerstag, 16. Januar 2020 19:07:48 CET Greg Kurz wrote:
> > The point here was that there must be some kind of absolute minimum msize,
> 
> Then the absolute minimum size is 7, the size of the header part that is
> common to all messages:
> 
> size[4] Message tag[2]
> 
> > even though the protocol specs officially don't mandate one. And if a
> > client choses a msize < P9_IOHDRSZ, what useful actions shall it be able
> > to do?
> I haven't checked but I guess some messages can fit in 24 bytes.
> 
> > That's why I mentioned P9_IOHDRSZ just in case somebody might come up
> > later
> > asking to lower that minimum msize value for whatever reason.
> 
> That's precisely what I want to avoid. The semantic of P9_IOHDRSZ is
> that it is the header size of a Twrite message and as such it is used
> to compute the 'iounit' which the guest later uses to compute a
> suitable 'count' for Tread/Twrite messages only. It shouldn't be
> involved in msize considerations IMHO.
> 
> > But np, IMO this sentence makes the fundamental requirement for this patch
> > more clear, but I have no problem dropping this sentence.
> 
> Then you can mention 7 has the true minimal size.

Ok, I'll drop P9_IOHDRSZ completely and mention literal 7 as absolute 
theoretical minimum instead.

> > > > Furthermore there are some responses which are not allowed by the 9p
> > > > protocol to be truncated like e.g. Rreadlink which may yield up to
> > > 
> > > No message may be truncated in any way actually. The spec just allows
> > > an exception with the string part of Rerror.
> > 
> > Rreaddir, Rread, Twrite, Rerror can be truncated. So I suggest I'll just
> > change that to s/some/most/ semantically.
> 
> I mean truncate with loss of data. Rreaddir can return less than 'count'
> but it won't truncate an individual entry. It rewinds to the previous
> one and returns its offset for the next Treaddir. Same goes with Rread
> and Twrite, we always return a 'count' that can be used by the client
> to continue reading or writing.

Individual entries are not truncated, correct, but data loss: Example, you 
have this directory and say server's fs would deliver entries by readdir() in 
this order (from top down):

.
..
a
1234567890
b
c
d

and say 37 >= msize < 45, then client would receive 3 entries ".", "..", "a" 
and that's it.

> Rerror is really the only message where we can deliberately drop
> data (but we actually don't do that).
> 
> > > Maybe just mention that and say we choose 4096 to be able to send
> > > big Rreadlink messages.
> > 
> > That's not the point for this patch. The main purpose is to avoid having
> > to
> > maintain individual error prone minimum msize checks all over the code
> > base. If we would allow very small msizes (much smaller than 4096), then
> > we would need to add numerous error handlers all over the code base, each
> > one checking for different values (depending on the specific message
> > type).
> 
> I'm not sure what you mean by 'minimum msize checks all over the code
> base'... Please provide an example.

1. Like done in this patch series: Treaddir has to limit client's 'count'
   parameter according to msize (by subtracting with msize).

2. get_iounit() does this:

iounit = stbuf.f_bsize;
iounit *= (s->msize - P9_IOHDRSZ) / stbuf.f_bsize;

   without checking anywhere for a potential negative outcome
   (i.e. when msize < P9_IOHDRSZ)

3. Example of directory entry name length with Rreaddir above.

Individual issues that can easily be overlooked but also easily avoided by not 
allowing small msizes in the first place.

Best regards,
Christian Schoenebeck

Re: [PATCH] qapi: Fix code generation with Python 3.5

2020-01-16 Thread John Snow




On 1/16/20 3:25 PM, Markus Armbruster wrote:
> Recent commit 3e7fb5811b "qapi: Fix code generation for empty modules"
> modules" switched QAPISchema.visit() from
> 
> for entity in self._entity_list:
> 
> effectively to
> 
> for mod in self._module_dict.values():
> for entity in mod._entity_list:
> 
> Visits in the same order as long as .values() is in insertion order.
> That's the case only for Python 3.6 and later.  Before, it's in some
> arbitrary order, which results in broken generated code.
> 
> Fix by making self._module_dict an OrderedDict rather than a dict.
> 
> Fixes: 3e7fb5811baab213dcc7149c3aa69442d683c26c
> Signed-off-by: Markus Armbruster 
> ---
>  scripts/qapi/schema.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 0bfc5256fb..5100110fa2 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -795,7 +795,7 @@ class QAPISchema(object):
>  self.docs = parser.docs
>  self._entity_list = []
>  self._entity_dict = {}
> -self._module_dict = {}
> +self._module_dict = OrderedDict()
>  self._schema_dir = os.path.dirname(fname)
>  self._make_module(None) # built-ins
>  self._make_module(fname)
> 

This problem has bitten me *many* times. I'm wondering if there's a
prescription that isn't just "Wait until we can stipulate 3.6+".

Re: qemu-4.0.1: vhost_region_add_section:Section rounded to 0 prior to previous a0000

2020-01-16 Thread Peter Lieven

> Am 16.01.2020 um 21:26 schrieb Dr. David Alan Gilbert :
> 
> * Peter Lieven (p...@kamp.de) wrote:
>> Am 16.01.20 um 13:47 schrieb Peter Lieven:
>>> Am 13.01.20 um 17:25 schrieb Peter Lieven:
 Am 09.01.20 um 19:44 schrieb Dr. David Alan Gilbert:
> * Peter Lieven (p...@kamp.de) wrote:
>> Am 08.01.20 um 16:04 schrieb Dr. David Alan Gilbert:
>>> * Peter Lieven (p...@kamp.de) wrote:
 Hi,

 I have a Qemu 4.0.1 machine with vhost-net network adapter, thats 
 polluting the log with the above message.

 Is this something known? Googling revealed the following patch in Nemu 
 (with seems to be a Qemu fork from Intel):

 https://github.com/intel/nemu/commit/03940ded7f5370ce7492c619dccced114ef7f56e

 The network stopped functioning. After a live-migration the vServer is 
 reachable again.

 Any ideas?
>>> What guest are you running and what does your qemu commandline look
>>> like?
>> 
>> Its running debian9. We have hundreds of other VMs with identical setup. 
>> Do not know why this one makes trouble.
> Could you extract an 'info mtree' from it - particularly the
> 'address-space: memory' near the top.

 Here we go:

 address-space: memory
   - (prio 0, i/o): system
 -3fff (prio 0, i/o): alias ram-below-4g 
 @pc.ram -3fff
 - (prio -1, i/o): pci
   000a-000a (prio 2, i/o): alias vga.chain4 
 @vga.vram -
   000a-000b (prio 1, i/o): vga-lowmem
>>> 
>>> 
>>> What seems special is that the RAM area is prio2. Any idea if this makes 
>>> trouble?
>> 
>> 
>> Update from my side. This happens when I have Debian 10 with XFCE when the 
>> Graphical User Interface is initialized.
>> 
>> I see the log message when I specify -M pc-i440fx-2.9. If I obmit the 
>> machine type the error does not appear.
> 
> I can't persuade this to reproduce here on the images I currently have;
> but if you can rebuild, can you try the v3 of 'Fix hyperv synic on
> vhost' I've just posted?  It turns off the alignment code that's
> spitting that error in vhost-kernel cases, so should go away.

I will definitely give it a try tomorrow. The fix to change the machine type 
was not working. I was too fast with my assumption.

What I see is the following:

Machine boots up (cold start), has network connectivity. As soon as the 
Graphics are initialized, the vhost_region_add_section error pops up and
Networking is interrupted. When I then migrate the vServer to another host 
Networking works again. It even seems to keep working when I do a
soft reset (shutdown -r). The only thing that breaks networking again when the 
graphic is initialized is a hard reset (system reset via hmp or amp).

Thank you,
Peter

[PATCH v2 2/4] vl: Reduce scope of variables in configure_accelerators

The accel_list and tmp variables are only used when manufacturing
-machine accel, options based on -accel.

Acked-by: Paolo Bonzini 
Reviewed-by: Alex Bennée 
Reviewed by: Aleksandar Markovic 
Signed-off-by: Richard Henderson 
---
v2: The freeing of accel_list was fixed in adb464ff671d.
---
 vl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index 6a5abf2f54..3e2b77a4e8 100644
--- a/vl.c
+++ b/vl.c
@@ -2753,7 +2753,6 @@ static int do_configure_accelerator(void *opaque, 
QemuOpts *opts, Error **errp)
 static void configure_accelerators(const char *progname)
 {
 const char *accel;
-char **accel_list, **tmp;
 bool init_failed = false;
 
 qemu_opts_foreach(qemu_find_opts("icount"),
@@ -2761,6 +2760,8 @@ static void configure_accelerators(const char *progname)
 
 accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
 if (QTAILQ_EMPTY(&qemu_accel_opts.head)) {
+char **accel_list, **tmp;
+
 if (accel == NULL) {
 /* Select the default accelerator */
 if (!accel_find("tcg") && !accel_find("kvm")) {
-- 
2.20.1

[PATCH v2 3/4] vl: Remove useless test in configure_accelerators

The result of g_strsplit is never NULL.

Acked-by: Paolo Bonzini 
Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed by: Aleksandar Markovic 
Signed-off-by: Richard Henderson 
---
 vl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index 3e2b77a4e8..8ae8a5d241 100644
--- a/vl.c
+++ b/vl.c
@@ -2781,7 +2781,7 @@ static void configure_accelerators(const char *progname)
 
 accel_list = g_strsplit(accel, ":", 0);
 
-for (tmp = accel_list; tmp && *tmp; tmp++) {
+for (tmp = accel_list; *tmp; tmp++) {
 /*
  * Filter invalid accelerators here, to prevent obscenities
  * such as "-machine accel=tcg,,thread=single".
-- 
2.20.1

[PATCH v2 4/4] vl: Only choose enabled accelerators in configure_accelerators

By choosing "tcg:kvm" when kvm is not enabled, we generate
an incorrect warning: "invalid accelerator kvm".

At the same time, use g_str_has_suffix rather than open-coding
the same operation.

Presumably the inverse is also true with --disable-tcg.

Fixes: 28a0961757fc
Acked-by: Paolo Bonzini 
Reviewed-by: Alex Bennée 
Reviewed by: Aleksandar Markovic 
Signed-off-by: Richard Henderson 
---
v2: Use g_str_has_suffix (ajb)
---
 vl.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/vl.c b/vl.c
index 8ae8a5d241..40ac9c5544 100644
--- a/vl.c
+++ b/vl.c
@@ -2764,21 +2764,26 @@ static void configure_accelerators(const char *progname)
 
 if (accel == NULL) {
 /* Select the default accelerator */
-if (!accel_find("tcg") && !accel_find("kvm")) {
-error_report("No accelerator selected and"
- " no default accelerator available");
-exit(1);
-} else {
-int pnlen = strlen(progname);
-if (pnlen >= 3 && g_str_equal(&progname[pnlen - 3], "kvm")) {
+bool have_tcg = accel_find("tcg");
+bool have_kvm = accel_find("kvm");
+
+if (have_tcg && have_kvm) {
+if (g_str_has_suffix(progname, "kvm")) {
 /* If the program name ends with "kvm", we prefer KVM */
 accel = "kvm:tcg";
 } else {
 accel = "tcg:kvm";
 }
+} else if (have_kvm) {
+accel = "kvm";
+} else if (have_tcg) {
+accel = "tcg";
+} else {
+error_report("No accelerator selected and"
+ " no default accelerator available");
+exit(1);
 }
 }
-
 accel_list = g_strsplit(accel, ":", 0);
 
 for (tmp = accel_list; *tmp; tmp++) {
-- 
2.20.1

[PATCH v2 1/4] vl: Remove unused variable in configure_accelerators

The accel_initialised variable no longer has any setters.

Fixes: 6f6e1698a68c
Acked-by: Paolo Bonzini 
Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed by: Aleksandar Markovic 
Signed-off-by: Richard Henderson 
---
 vl.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 751401214c..6a5abf2f54 100644
--- a/vl.c
+++ b/vl.c
@@ -2754,7 +2754,6 @@ static void configure_accelerators(const char *progname)
 {
 const char *accel;
 char **accel_list, **tmp;
-bool accel_initialised = false;
 bool init_failed = false;
 
 qemu_opts_foreach(qemu_find_opts("icount"),
@@ -2781,7 +2780,7 @@ static void configure_accelerators(const char *progname)
 
 accel_list = g_strsplit(accel, ":", 0);
 
-for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
+for (tmp = accel_list; tmp && *tmp; tmp++) {
 /*
  * Filter invalid accelerators here, to prevent obscenities
  * such as "-machine accel=tcg,,thread=single".
-- 
2.20.1

[PATCH v2 0/4] vl: Fixes for cleanups to -accel

Running qemu-system-foo with no options should not generate
a warning for "invalid accelerator bar".

Changes in v2:
  * Rebase on master, getting the free accel_list fix from upstream.
Re-word the resulting patch 2 to merely reduce the scope of the
local variables.
  * Use g_str_has_suffix (ajb)


r~


Richard Henderson (4):
  vl: Remove unused variable in configure_accelerators
  vl: Reduce scope of variables in configure_accelerators
  vl: Remove useless test in configure_accelerators
  vl: Only choose enabled accelerators in configure_accelerators

 vl.c | 27 ---
 1 file changed, 16 insertions(+), 11 deletions(-)

-- 
2.20.1

[Bug 1859713] Re: ARM v8.3a pauth not working

2020-01-16 Thread Vincent Dehors

Hi,

Here is a patch for this bug. The sbox function was using "b+=16"
instead of "b+=4".

Also, you check test vector using :

```c
uint64_t P = 0xfb623599da6e8127ull;
uint64_t T = 0x477d469dec0b8762ull;
uint64_t w0 = 0x84be85ce9804e94bull;
uint64_t k0 = 0xec2802d4e0a488e9ull;
ARMPACKey key = { .hi = w0, .lo = k0 };
uint64_t C5 = pauth_computepac(P, T, key);
/* C5 should be 0xc003b93999b33765 */
```

** Patch added: "0001-target-arm-Fix-PAuth-sbox-functions.patch"
   
https://bugs.launchpad.net/qemu/+bug/1859713/+attachment/5320937/+files/0001-target-arm-Fix-PAuth-sbox-functions.patch

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

Title:
  ARM v8.3a pauth not working

Status in QEMU:
  Confirmed

Bug description:
  Host: Ubuntu 19.10 - x86_64 machine
  QEMU version: 3a63b24a1bbf166e6f455fe43a6bbd8dea413d92 (master)

  ARMV8.3 pauth is not working well.

  With a test code containing two pauth instructions:
  - paciasp that sign LR with A key and sp as context;
  - autiasp that verify the signature.

  Test:
  - Run the program and corrupt LR just before autiasp (ex 0x3e0400660 
instead of 0x3e00400664)

  Expected:
  - autiasp places an invalid pointer in LR

  Result:
  - autiasp successfully auth the pointer and places 0x0400660 in LR.

  Further explanations:
  Adding traces in qemu code shows that "pauth_computepac" is not robust 
enough against truncating.
  With 0x3100400664 as input of pauth_auth, we obtain 
"0x55b1d65b2c138e14" for PAC, "0x30" for bot_bit and "0x38" for top_bit.
  With 0x310040008743ec as input of pauth (with same key), we obtain 
"0x55b1d65b2c138ef4" for PAC, "0x30" for bot_bit and "0x38" for top_bit.
  Values of top_bit and bottom_bit are strictly the same and it should not.

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

Re: [PATCH] target/s390x/kvm: Enable adapter interruption suppression again

2020-01-16 Thread Cornelia Huck

On Thu, 16 Jan 2020 15:19:13 -0500
Matthew Rosato  wrote:

> On 1/16/20 7:20 AM, Thomas Huth wrote:
> > The AIS feature has been disabled late in the v2.10 development
> > cycle since there were some issues with migration (see commit
> > 3f2d07b3b01ea61126b - "s390x/ais: for 2.10 stable: disable ais
> > facility"). We originally wanted to enable it again for newer
> > machine types, but apparently we forgot to do this so far. Let's
> > do it for the new s390-ccw-virtio-5.0 machine now.
> > 
> > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1756946
> > Signed-off-by: Thomas Huth 
> > ---
> >   hw/s390x/s390-virtio-ccw.c |  4 
> >   include/hw/s390x/s390-virtio-ccw.h |  4 
> >   target/s390x/kvm.c | 11 ---
> >   3 files changed, 16 insertions(+), 3 deletions(-)

(...)

> > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> > index 15260aeb9a..4c1c8c0208 100644
> > --- a/target/s390x/kvm.c
> > +++ b/target/s390x/kvm.c
> > @@ -329,6 +329,8 @@ static void ccw_machine_class_foreach(ObjectClass *oc, 
> > void *opaque)
> >   
> >   int kvm_arch_init(MachineState *ms, KVMState *s)
> >   {
> > +S390CcwMachineClass *smc = S390_CCW_MACHINE_OBJ_GET_CLASS(ms);
> > +  
> 
> I still can't run a proper test due to unavailable hw but in the 
> meantime I tried to virsh define a libvirt guest pointed at qemu (master 
> + this patch).  Regardless of machine type (s390-ccw-virtio-5.0 or 
> s390-ccw-virtio-4.2) I get:
> 
> virsh define guest.xml
> error: Failed to define domain from /path/to/guest.xml
> error: invalid argument: could not find capabilities for arch=s390x 
> domaintype=kvm
> 
> Similarly:
> 
> virsh domcapabilities
> error: failed to get emulator capabilities
> error: invalid argument: unable to find any emulator to serve 's390x' 
> architecture
> 
> Rolling back to qemu master, the define and domcapabilities work (with 
> no ais of course).
> 
> So: there is some incompatibility between the way libvirt invokes qemu 
> to detect capabilities and this code.  The above line seems to be the 
> root problem - if I take your patch and remove 'smc' then libvirt works 
> as expected and I can see ais in the domcapabilities.
> 
> Looking at those wrappers David mentioned...  I suspect you need this 
> for the 'none' machine case.  I tried a quick hack with the following:
> 
> bool ais_allowed(void)
> {
>  /* for "none" machine this results in true */
>  return get_machine_class()->kvm_ais_allowed;
> }
> 
> and
> 
> if (ais_allowed() &&
>  kvm_check_extension(s, KVM_CAP_S390_AIS_MIGRATION)) {
>  kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0);
> }
> 
> This works and doesn't break libvirt compatibility detection.

Oh, "none" machine fun again... I think you're on the right track, and
we really need a wrapper.

Re: qemu-4.0.1: vhost_region_add_section:Section rounded to 0 prior to previous a0000

2020-01-16 Thread Dr. David Alan Gilbert

* Peter Lieven (p...@kamp.de) wrote:
> Am 16.01.20 um 13:47 schrieb Peter Lieven:
> > Am 13.01.20 um 17:25 schrieb Peter Lieven:
> > > Am 09.01.20 um 19:44 schrieb Dr. David Alan Gilbert:
> > > > * Peter Lieven (p...@kamp.de) wrote:
> > > > > Am 08.01.20 um 16:04 schrieb Dr. David Alan Gilbert:
> > > > > > * Peter Lieven (p...@kamp.de) wrote:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > 
> > > > > > > I have a Qemu 4.0.1 machine with vhost-net network adapter, thats 
> > > > > > > polluting the log with the above message.
> > > > > > > 
> > > > > > > Is this something known? Googling revealed the following patch in 
> > > > > > > Nemu (with seems to be a Qemu fork from Intel):
> > > > > > > 
> > > > > > > https://github.com/intel/nemu/commit/03940ded7f5370ce7492c619dccced114ef7f56e
> > > > > > > 
> > > > > > > 
> > > > > > > The network stopped functioning. After a live-migration the 
> > > > > > > vServer is reachable again.
> > > > > > > 
> > > > > > > 
> > > > > > > Any ideas?
> > > > > > What guest are you running and what does your qemu commandline look
> > > > > > like?
> > > > > 
> > > > > Its running debian9. We have hundreds of other VMs with identical 
> > > > > setup. Do not know why this one makes trouble.
> > > > Could you extract an 'info mtree' from it - particularly the
> > > > 'address-space: memory' near the top.
> > > 
> > > 
> > > Here we go:
> > > 
> > > 
> > > address-space: memory
> > >   - (prio 0, i/o): system
> > >     -3fff (prio 0, i/o): alias ram-below-4g 
> > > @pc.ram -3fff
> > >     - (prio -1, i/o): pci
> > >   000a-000a (prio 2, i/o): alias vga.chain4 
> > > @vga.vram -
> > >   000a-000b (prio 1, i/o): vga-lowmem
> > 
> > 
> > What seems special is that the RAM area is prio2. Any idea if this makes 
> > trouble?
> 
> 
> Update from my side. This happens when I have Debian 10 with XFCE when the 
> Graphical User Interface is initialized.
> 
> I see the log message when I specify -M pc-i440fx-2.9. If I obmit the machine 
> type the error does not appear.

I can't persuade this to reproduce here on the images I currently have;
but if you can rebuild, can you try the v3 of 'Fix hyperv synic on
vhost' I've just posted?  It turns off the alignment code that's
spitting that error in vhost-kernel cases, so should go away.

Dave

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

[PATCH] qapi: Fix code generation with Python 3.5

Recent commit 3e7fb5811b "qapi: Fix code generation for empty modules"
modules" switched QAPISchema.visit() from

for entity in self._entity_list:

effectively to

for mod in self._module_dict.values():
for entity in mod._entity_list:

Visits in the same order as long as .values() is in insertion order.
That's the case only for Python 3.6 and later.  Before, it's in some
arbitrary order, which results in broken generated code.

Fix by making self._module_dict an OrderedDict rather than a dict.

Fixes: 3e7fb5811baab213dcc7149c3aa69442d683c26c
Signed-off-by: Markus Armbruster 
---
 scripts/qapi/schema.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 0bfc5256fb..5100110fa2 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -795,7 +795,7 @@ class QAPISchema(object):
 self.docs = parser.docs
 self._entity_list = []
 self._entity_dict = {}
-self._module_dict = {}
+self._module_dict = OrderedDict()
 self._schema_dir = os.path.dirname(fname)
 self._make_module(None) # built-ins
 self._make_module(fname)
-- 
2.21.1

[PATCH v3 2/2] vhost: Only align sections for vhost-user

2020-01-16 Thread Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" 

I added hugepage alignment code in c1ece84e7c9 to deal with
vhost-user + postcopy which needs aligned pages when using userfault.
However, on x86 the lower 2MB of address space tends to be shotgun'd
with small fragments around the 512-640k range - e.g. video RAM, and
with HyperV synic pages tend to sit around there - again splitting
it up.  The alignment code complains with a 'Section rounded to ...'
error and gives up.

Since vhost-user already filters out devices without an fd
(see vhost-user.c vhost_user_mem_section_filter) it shouldn't be
affected by those overlaps.

Turn the alignment off on vhost-kernel so that it doesn't try
and align, and thus won't hit the rounding issues.

Signed-off-by: Dr. David Alan Gilbert 
---
 hw/virtio/vhost.c | 34 ++
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 774d87d98e..25fd469179 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -547,26 +547,28 @@ static void vhost_region_add_section(struct vhost_dev 
*dev,
 uintptr_t mrs_host = (uintptr_t)memory_region_get_ram_ptr(section->mr) +
  section->offset_within_region;
 RAMBlock *mrs_rb = section->mr->ram_block;
-size_t mrs_page = qemu_ram_pagesize(mrs_rb);
 
 trace_vhost_region_add_section(section->mr->name, mrs_gpa, mrs_size,
mrs_host);
 
-/* Round the section to it's page size */
-/* First align the start down to a page boundary */
-uint64_t alignage = mrs_host & (mrs_page - 1);
-if (alignage) {
-mrs_host -= alignage;
-mrs_size += alignage;
-mrs_gpa  -= alignage;
-}
-/* Now align the size up to a page boundary */
-alignage = mrs_size & (mrs_page - 1);
-if (alignage) {
-mrs_size += mrs_page - alignage;
-}
-trace_vhost_region_add_section_aligned(section->mr->name, mrs_gpa, 
mrs_size,
-   mrs_host);
+if (dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER) {   
+/* Round the section to it's page size */
+/* First align the start down to a page boundary */
+size_t mrs_page = qemu_ram_pagesize(mrs_rb);
+uint64_t alignage = mrs_host & (mrs_page - 1);
+if (alignage) {
+mrs_host -= alignage;
+mrs_size += alignage;
+mrs_gpa  -= alignage;
+}
+/* Now align the size up to a page boundary */
+alignage = mrs_size & (mrs_page - 1);
+if (alignage) {
+mrs_size += mrs_page - alignage;
+}
+trace_vhost_region_add_section_aligned(section->mr->name, mrs_gpa, 
mrs_size,
+   mrs_host);
+}
 
 if (dev->n_tmp_sections) {
 /* Since we already have at least one section, lets see if
-- 
2.24.1

[PATCH v3 0/2] Fix hyperv synic on vhost

2020-01-16 Thread Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" 

Hyperv's synic (that we emulate) is a feature that allows the guest
to place some magic (4k) pages of RAM anywhere it likes in GPA.
This confuses vhost's RAM section merging when these pages
land over the top of hugepages.

This v3 takes a different approach to v2 and v1.
It avoids doing the hugepage alignment except for vhost-user:

  a) Vhost kernel : doesn't need alignment, it's turned off
 synic won't cause a problem.

  b) vhost user : Already filters out anything without an fd
 synic won't cause a problem.

(Not tried vhost-user yet, it currently seems broken even without this).

This might also cause some other reported problems with vga
pages causing similar issues.

bz: https://bugzilla.redhat.com/show_bug.cgi?id=1779041

Dr. David Alan Gilbert (2):
  vhost: Add names to section rounded warning
  vhost: Only align sections for vhost-user

 hw/virtio/vhost.c | 39 +--
 1 file changed, 21 insertions(+), 18 deletions(-)

-- 
2.24.1

[PATCH v3 1/2] vhost: Add names to section rounded warning

2020-01-16 Thread Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" 

Add the memory region names to section rounding/alignment
warnings.

Signed-off-by: Dr. David Alan Gilbert 
---
 hw/virtio/vhost.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 4da0d5a6c5..774d87d98e 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -590,9 +590,10 @@ static void vhost_region_add_section(struct vhost_dev *dev,
  * match up in the same RAMBlock if they do.
  */
 if (mrs_gpa < prev_gpa_start) {
-error_report("%s:Section rounded to %"PRIx64
- " prior to previous %"PRIx64,
- __func__, mrs_gpa, prev_gpa_start);
+error_report("%s:Section '%s' rounded to %"PRIx64
+ " prior to previous '%s' %"PRIx64,
+ __func__, section->mr->name, mrs_gpa,
+ prev_sec->mr->name, prev_gpa_start);
 /* A way to cleanly fail here would be better */
 return;
 }
-- 
2.24.1

Re: [PATCH] target/s390x/kvm: Enable adapter interruption suppression again

2020-01-16 Thread Matthew Rosato


On 1/16/20 7:20 AM, Thomas Huth wrote:

The AIS feature has been disabled late in the v2.10 development
cycle since there were some issues with migration (see commit
3f2d07b3b01ea61126b - "s390x/ais: for 2.10 stable: disable ais
facility"). We originally wanted to enable it again for newer
machine types, but apparently we forgot to do this so far. Let's
do it for the new s390-ccw-virtio-5.0 machine now.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1756946
Signed-off-by: Thomas Huth 
---
  hw/s390x/s390-virtio-ccw.c |  4 
  include/hw/s390x/s390-virtio-ccw.h |  4 
  target/s390x/kvm.c | 11 ---
  3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index e7eadd14e8..6f43136396 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -456,6 +456,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void 
*data)
  s390mc->cpu_model_allowed = true;
  s390mc->css_migration_enabled = true;
  s390mc->hpage_1m_allowed = true;
+s390mc->kvm_ais_allowed = true;
  mc->init = ccw_init;
  mc->reset = s390_machine_reset;
  mc->hot_add_cpu = s390_hot_add_cpu;
@@ -662,6 +663,9 @@ static void ccw_machine_4_2_instance_options(MachineState 
*machine)
  
  static void ccw_machine_4_2_class_options(MachineClass *mc)

  {
+S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
+
+s390mc->kvm_ais_allowed = false;
  ccw_machine_5_0_class_options(mc);
  compat_props_add(mc->compat_props, hw_compat_4_2, hw_compat_4_2_len);
  }
diff --git a/include/hw/s390x/s390-virtio-ccw.h 
b/include/hw/s390x/s390-virtio-ccw.h
index 8aa27199c9..f142d379c6 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -21,6 +21,9 @@
  #define S390_MACHINE_CLASS(klass) \
  OBJECT_CLASS_CHECK(S390CcwMachineClass, (klass), TYPE_S390_CCW_MACHINE)
  
+#define S390_CCW_MACHINE_OBJ_GET_CLASS(obj) \

+OBJECT_GET_CLASS(S390CcwMachineClass, obj, TYPE_S390_CCW_MACHINE)
+
  typedef struct S390CcwMachineState {
  /*< private >*/
  MachineState parent_obj;
@@ -40,6 +43,7 @@ typedef struct S390CcwMachineClass {
  bool cpu_model_allowed;
  bool css_migration_enabled;
  bool hpage_1m_allowed;
+bool kvm_ais_allowed;
  } S390CcwMachineClass;
  
  /* runtime-instrumentation allowed by the machine */

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 15260aeb9a..4c1c8c0208 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -329,6 +329,8 @@ static void ccw_machine_class_foreach(ObjectClass *oc, void 
*opaque)
  
  int kvm_arch_init(MachineState *ms, KVMState *s)

  {
+S390CcwMachineClass *smc = S390_CCW_MACHINE_OBJ_GET_CLASS(ms);
+


I still can't run a proper test due to unavailable hw but in the 
meantime I tried to virsh define a libvirt guest pointed at qemu (master 
+ this patch).  Regardless of machine type (s390-ccw-virtio-5.0 or 
s390-ccw-virtio-4.2) I get:


virsh define guest.xml
error: Failed to define domain from /path/to/guest.xml
error: invalid argument: could not find capabilities for arch=s390x 
domaintype=kvm


Similarly:

virsh domcapabilities
error: failed to get emulator capabilities
error: invalid argument: unable to find any emulator to serve 's390x' 
architecture


Rolling back to qemu master, the define and domcapabilities work (with 
no ais of course).


So: there is some incompatibility between the way libvirt invokes qemu 
to detect capabilities and this code.  The above line seems to be the 
root problem - if I take your patch and remove 'smc' then libvirt works 
as expected and I can see ais in the domcapabilities.


Looking at those wrappers David mentioned...  I suspect you need this 
for the 'none' machine case.  I tried a quick hack with the following:


bool ais_allowed(void)
{
/* for "none" machine this results in true */
return get_machine_class()->kvm_ais_allowed;
}

and

if (ais_allowed() &&
kvm_check_extension(s, KVM_CAP_S390_AIS_MIGRATION)) {
kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0);
}

This works and doesn't break libvirt compatibility detection.




  object_class_foreach(ccw_machine_class_foreach, TYPE_S390_CCW_MACHINE,
   false, NULL);
  
@@ -365,10 +367,13 @@ int kvm_arch_init(MachineState *ms, KVMState *s)

  /*
   * The migration interface for ais was introduced with kernel 4.13
   * but the capability itself had been active since 4.12. As migration
- * support is considered necessary let's disable ais in the 2.10
- * machine.
+ * support is considered necessary we only enable this for newer
+ * machine types and if KVM_CAP_S390_AIS_MIGRATION is available.
   */
-/* kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0); */
+if (smc->kvm_ais_allowed &&
+kvm_check_extension(s, KVM_CAP_S390_AIS_MIGRATION)) {
+kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0);
+}
  
  kvm_set_max_me

Re: [PATCH 3/3] linux-user/i386: Emulate x86_64 vsyscalls

2020-01-16 Thread Alex Bennée



Richard Henderson  writes:

> On 1/16/20 6:26 AM, Alex Bennée wrote:
>>> +/*
>>> + * Perform the syscall.  None of the vsyscalls should need restarting,
>>> + * and all faults should have been caught above.
>>> + */
>>> +ret = do_syscall(env, syscall, env->regs[R_EDI], env->regs[R_ESI],
>>> + env->regs[R_EDX], env->regs[10], env->regs[8],
>>> + env->regs[9], 0, 0);
>> 
>> How come the register ABI to the syscall is different to the others. I
>> can see why syscall doesn't come from EAX but the others are a different
>> set to normal syscalls which might be why:
>
> Cut and paste error, I assume.
>
> That said, the three syscalls have a maximum of 2 arguments,
> so I could really just pass EDI and ESI and 0 for the rest...
>
>> I'm seeing a EFAULT on the gettimeofday failure:
>
> What getttimeofday failure?  Is this related to the mention of /sbin/ldconfig
> in your previous message?

No - the buster x86064 ldconfig seg is unrelated to this series. It has
however spawned an additional bug in gdbstub while it was at it ;-)

>
>>#0  do_syscall (cpu_env=cpu_env@entry=0x577d2b10, num=num@entry=96, 
>> arg1=0, arg2=0, arg3=4211016, arg4=8, arg5=274888677184, arg6=274886295415, 
>> arg7=0, arg8=0) at /home/alex/lsrc/qemu.git/linux-user/syscall.c:12076   
>> 
>>#1  0x55609b6e in emulate_vsyscall (env=0x577d2b10) at 
>> /home/alex/lsrc/qemu.git/linux-user/x86_64/../i386/cpu_loop.c:180
>>#2  cpu_loop (env=0x577d2b10) at 
>> /home/alex/lsrc/qemu.git/linux-user/x86_64/../i386/cpu_loop.c:246
>>   
>>#3  0x5559640e in main (argc=, argv=>#out>, envp=) at
>>#/home/alex/lsrc/qemu.git/linux-user/main.c:865
>> 
>> arg1/arg2 don't seem right here.
>
> Why?  NULL value for arg1 is legal, though semi-useless.
>
> Ah, I see that our implementation of gettimeofday doesn't honor NULL.
>
>
> r~


-- 
Alex Bennée

[PATCH v2 3/5] linux-user/i386: Emulate x86_64 vsyscalls

Notice the magic page during translate, much like we already
do for the arm32 commpage.  At runtime, raise an exception to
return cpu_loop for emulation.

Reviewed-by: Paolo Bonzini 
Signed-off-by: Richard Henderson 
---
 target/i386/cpu.h  |   1 +
 linux-user/i386/cpu_loop.c | 105 +
 target/i386/translate.c|  16 +-
 3 files changed, 121 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 164d038d1f..3fb2d2a986 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1000,6 +1000,7 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
 
 #define EXCP_VMEXIT 0x100 /* only for system emulation */
 #define EXCP_SYSCALL0x101 /* only for user emulation */
+#define EXCP_VSYSCALL   0x102 /* only for user emulation */
 
 /* i386-specific interrupt pending bits.  */
 #define CPU_INTERRUPT_POLL  CPU_INTERRUPT_TGT_EXT_1
diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
index e217cca5ee..f9bf6cec27 100644
--- a/linux-user/i386/cpu_loop.c
+++ b/linux-user/i386/cpu_loop.c
@@ -92,6 +92,106 @@ static void gen_signal(CPUX86State *env, int sig, int code, 
abi_ptr addr)
 queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
 }
 
+#ifdef TARGET_X86_64
+static bool write_ok_or_segv(CPUX86State *env, abi_ptr addr, size_t len)
+{
+/*
+ * For all the vsyscalls, NULL means "don't write anything" not
+ * "write it at address 0".
+ */
+if (addr == 0 || access_ok(VERIFY_WRITE, addr, len)) {
+return true;
+}
+
+env->error_code = PG_ERROR_W_MASK | PG_ERROR_U_MASK;
+gen_signal(env, TARGET_SIGSEGV, TARGET_SEGV_MAPERR, addr);
+return false;
+}
+
+/*
+ * Since v3.1, the kernel traps and emulates the vsyscall page.
+ * Entry points other than the official generate SIGSEGV.
+ */
+static void emulate_vsyscall(CPUX86State *env)
+{
+int syscall;
+abi_ulong ret;
+uint64_t caller;
+
+/*
+ * Validate the entry point.  We have already validated the page
+ * during translation, now verify the offset.
+ */
+switch (env->eip & ~TARGET_PAGE_MASK) {
+case 0x000:
+syscall = TARGET_NR_gettimeofday;
+break;
+case 0x400:
+syscall = TARGET_NR_time;
+break;
+case 0x800:
+syscall = TARGET_NR_getcpu;
+break;
+default:
+sigsegv:
+/* Like force_sig(SIGSEGV).  */
+gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0);
+return;
+}
+
+/*
+ * Validate the return address.
+ * Note that the kernel treats this the same as an invalid entry point.
+ */
+if (get_user_u64(caller, env->regs[R_ESP])) {
+goto sigsegv;
+}
+
+/*
+ * Validate the the pointer arguments.
+ */
+switch (syscall) {
+case TARGET_NR_gettimeofday:
+if (!write_ok_or_segv(env, env->regs[R_EDI],
+  sizeof(struct target_timeval)) ||
+!write_ok_or_segv(env, env->regs[R_ESI],
+  sizeof(struct target_timezone))) {
+return;
+}
+break;
+case TARGET_NR_time:
+if (!write_ok_or_segv(env, env->regs[R_EDI], sizeof(abi_long))) {
+return;
+}
+break;
+case TARGET_NR_getcpu:
+if (!write_ok_or_segv(env, env->regs[R_EDI], sizeof(uint32_t)) ||
+!write_ok_or_segv(env, env->regs[R_ESI], sizeof(uint32_t))) {
+return;
+}
+break;
+default:
+g_assert_not_reached();
+}
+
+/*
+ * Perform the syscall.  None of the vsyscalls should need restarting,
+ * and all faults should have been caught above.
+ */
+ret = do_syscall(env, syscall, env->regs[R_EDI], env->regs[R_ESI],
+ env->regs[R_EDX], env->regs[10], env->regs[8],
+ env->regs[9], 0, 0);
+g_assert(ret != -TARGET_ERESTARTSYS);
+g_assert(ret != -TARGET_QEMU_ESIGRETURN);
+g_assert(ret != -TARGET_EFAULT);
+env->regs[R_EAX] = ret;
+
+/* Emulate a ret instruction to leave the vsyscall page.  */
+env->eip = caller;
+env->regs[R_ESP] += 8;
+}
+#endif
+
 void cpu_loop(CPUX86State *env)
 {
 CPUState *cs = env_cpu(env);
@@ -141,6 +241,11 @@ void cpu_loop(CPUX86State *env)
 env->regs[R_EAX] = ret;
 }
 break;
+#endif
+#ifdef TARGET_X86_64
+case EXCP_VSYSCALL:
+emulate_vsyscall(env);
+break;
 #endif
 case EXCP0B_NOSEG:
 case EXCP0C_STACK:
diff --git a/target/i386/translate.c b/target/i386/translate.c
index 7c99ef1385..391b4ef149 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -8555,7 +8555,21 @@ static bool i386_tr_breakpoint_check(DisasContextBase 
*dcbase, CPUState *cpu,
 static void i386_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
 {
 DisasContext *dc = container_of(dcbase, DisasContext, base);
-target_ulong pc_next = disas

Re: [PATCH v2 0/5] linux-user: Implement x86_64 vsyscalls

On 1/16/20 9:43 AM, Richard Henderson wrote:
> Changes since v2:
> 
>   * Add /proc/self/maps line
> 
>   I'm not sure this is really necessary.  The linux kernel
>   self-test checks for it, and modifies the set of tests that
>   it runs based on it.  But otherwise I think it's unused.
> 
>   * Fix errors in base gettimeofday syscall
> 
>   This is also checked by test_vsyscall, as noticed by AJB.

Oh, and

* Set error_code in write_ok_or_segv (patch 2)


r~

[PATCH v2 4/5] linux-user: Add x86_64 vsyscall page to /proc/self/maps

The page isn't (necessarily) present in the host /proc/self/maps,
and even if it might be it isn't present in page_flags, and even
if it was it might not have the same set of page permissions.

The easiest thing to do, particularly when it comes to the
"[vsyscall]" note at the end of line, is to special case it.

Signed-off-by: Richard Henderson 
---
 linux-user/syscall.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 171c0caef3..eb867a5296 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7005,6 +7005,15 @@ static int open_self_maps(void *cpu_env, int fd)
 }
 }
 
+#ifdef TARGET_X86_64
+/*
+ * We only support execution from the vsyscall page.
+ * This is as if CONFIG_LEGACY_VSYSCALL_XONLY=y from v5.3.
+ */
+dprintf(fd, "ff60-ff601000 --xp  00:00 0"
+"  [vsyscall]\n");
+#endif
+
 free(line);
 fclose(fp);
 
-- 
2.20.1

[PATCH v2 1/5] target/i386: Renumber EXCP_SYSCALL

We are not short of numbers for EXCP_*.  There is no need to confuse things
by having EXCP_VMEXIT and EXCP_SYSCALL overlap, even though the former is
only used for system mode and the latter is only used for user mode.

Reviewed-by: Paolo Bonzini 
Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 target/i386/cpu.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 594326a794..164d038d1f 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -998,9 +998,8 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
 #define EXCP11_ALGN17
 #define EXCP12_MCHK18
 
-#define EXCP_SYSCALL0x100 /* only happens in user only emulation
- for syscall instruction */
-#define EXCP_VMEXIT 0x100
+#define EXCP_VMEXIT 0x100 /* only for system emulation */
+#define EXCP_SYSCALL0x101 /* only for user emulation */
 
 /* i386-specific interrupt pending bits.  */
 #define CPU_INTERRUPT_POLL  CPU_INTERRUPT_TGT_EXT_1
-- 
2.20.1

[PATCH v2 5/5] linux-user: Flush out implementation of gettimeofday

The first argument, timeval, is allowed to be NULL.

The second argument, timezone, was missing.  While its use is
deprecated, it is still present in the syscall.

Signed-off-by: Richard Henderson 
---
 linux-user/syscall.c | 27 +--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index eb867a5296..628b4de9a1 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1219,6 +1219,23 @@ static inline abi_long 
host_to_target_timespec64(abi_ulong target_addr,
 return 0;
 }
 
+static inline abi_long copy_to_user_timezone(abi_ulong target_tz_addr,
+ struct timezone *tz)
+{
+struct target_timezone *target_tz;
+
+if (!lock_user_struct(VERIFY_WRITE, target_tz, target_tz_addr, 1)) {
+return -TARGET_EFAULT;
+}
+
+__put_user(tz->tz_minuteswest, &target_tz->tz_minuteswest);
+__put_user(tz->tz_dsttime, &target_tz->tz_dsttime);
+
+unlock_user_struct(target_tz, target_tz_addr, 1);
+
+return 0;
+}
+
 static inline abi_long copy_from_user_timezone(struct timezone *tz,
abi_ulong target_tz_addr)
 {
@@ -8567,10 +8584,16 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 case TARGET_NR_gettimeofday:
 {
 struct timeval tv;
-ret = get_errno(gettimeofday(&tv, NULL));
+struct timezone tz;
+
+ret = get_errno(gettimeofday(&tv, &tz));
 if (!is_error(ret)) {
-if (copy_to_user_timeval(arg1, &tv))
+if (arg1 && copy_to_user_timeval(arg1, &tv)) {
 return -TARGET_EFAULT;
+}
+if (arg2 && copy_to_user_timezone(arg2, &tz)) {
+return -TARGET_EFAULT;
+}
 }
 }
 return ret;
-- 
2.20.1

[PATCH v2 2/5] linux-user/i386: Split out gen_signal

This is a bit tidier than open-coding the 5 lines necessary
to initialize the target_siginfo_t.  In addition, this zeros
the remaining bytes of the target_siginfo_t, rather than
passing in garbage.

Reviewed-by: Paolo Bonzini 
Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 linux-user/i386/cpu_loop.c | 93 ++
 1 file changed, 33 insertions(+), 60 deletions(-)

diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
index 024b6f4d58..e217cca5ee 100644
--- a/linux-user/i386/cpu_loop.c
+++ b/linux-user/i386/cpu_loop.c
@@ -81,13 +81,23 @@ static void set_idt(int n, unsigned int dpl)
 }
 #endif
 
+static void gen_signal(CPUX86State *env, int sig, int code, abi_ptr addr)
+{
+target_siginfo_t info = {
+.si_signo = sig,
+.si_code = code,
+._sifields._sigfault._addr = addr
+};
+
+queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+}
+
 void cpu_loop(CPUX86State *env)
 {
 CPUState *cs = env_cpu(env);
 int trapnr;
 abi_ulong pc;
 abi_ulong ret;
-target_siginfo_t info;
 
 for(;;) {
 cpu_exec_start(cs);
@@ -134,70 +144,45 @@ void cpu_loop(CPUX86State *env)
 #endif
 case EXCP0B_NOSEG:
 case EXCP0C_STACK:
-info.si_signo = TARGET_SIGBUS;
-info.si_errno = 0;
-info.si_code = TARGET_SI_KERNEL;
-info._sifields._sigfault._addr = 0;
-queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+gen_signal(env, TARGET_SIGBUS, TARGET_SI_KERNEL, 0);
 break;
 case EXCP0D_GPF:
 /* XXX: potential problem if ABI32 */
 #ifndef TARGET_X86_64
 if (env->eflags & VM_MASK) {
 handle_vm86_fault(env);
-} else
-#endif
-{
-info.si_signo = TARGET_SIGSEGV;
-info.si_errno = 0;
-info.si_code = TARGET_SI_KERNEL;
-info._sifields._sigfault._addr = 0;
-queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+break;
 }
+#endif
+gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0);
 break;
 case EXCP0E_PAGE:
-info.si_signo = TARGET_SIGSEGV;
-info.si_errno = 0;
-if (!(env->error_code & 1))
-info.si_code = TARGET_SEGV_MAPERR;
-else
-info.si_code = TARGET_SEGV_ACCERR;
-info._sifields._sigfault._addr = env->cr[2];
-queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+gen_signal(env, TARGET_SIGSEGV,
+   (env->error_code & 1 ?
+TARGET_SEGV_ACCERR : TARGET_SEGV_MAPERR),
+   env->cr[2]);
 break;
 case EXCP00_DIVZ:
 #ifndef TARGET_X86_64
 if (env->eflags & VM_MASK) {
 handle_vm86_trap(env, trapnr);
-} else
-#endif
-{
-/* division by zero */
-info.si_signo = TARGET_SIGFPE;
-info.si_errno = 0;
-info.si_code = TARGET_FPE_INTDIV;
-info._sifields._sigfault._addr = env->eip;
-queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+break;
 }
+#endif
+gen_signal(env, TARGET_SIGFPE, TARGET_FPE_INTDIV, env->eip);
 break;
 case EXCP01_DB:
 case EXCP03_INT3:
 #ifndef TARGET_X86_64
 if (env->eflags & VM_MASK) {
 handle_vm86_trap(env, trapnr);
-} else
+break;
+}
 #endif
-{
-info.si_signo = TARGET_SIGTRAP;
-info.si_errno = 0;
-if (trapnr == EXCP01_DB) {
-info.si_code = TARGET_TRAP_BRKPT;
-info._sifields._sigfault._addr = env->eip;
-} else {
-info.si_code = TARGET_SI_KERNEL;
-info._sifields._sigfault._addr = 0;
-}
-queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+if (trapnr == EXCP01_DB) {
+gen_signal(env, TARGET_SIGTRAP, TARGET_TRAP_BRKPT, env->eip);
+} else {
+gen_signal(env, TARGET_SIGTRAP, TARGET_SI_KERNEL, 0);
 }
 break;
 case EXCP04_INTO:
@@ -205,31 +190,19 @@ void cpu_loop(CPUX86State *env)
 #ifndef TARGET_X86_64
 if (env->eflags & VM_MASK) {
 handle_vm86_trap(env, trapnr);
-} else
-#endif
-{
-info.si_signo = TARGET_SIGSEGV;
-info.si_errno = 0;
-info.si_code = TARGET_SI_KERNEL;
-info._sifields._sigfault._addr = 0;
-queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+break;
 }
+#e

[PATCH v2 0/5] linux-user: Implement x86_64 vsyscalls