[Qemu-devel] [PATCH v2 3/3] s390x/pci: add iommu replay callback

[Yi Min Zhao]

Let's introduce iommu replay callback for s390 pci iommu memory region.
Currently we don't need any dma mapping replay. So let it return
directly. This implementation will avoid meaningless loops calling
translation callback.

Reviewed-by: Pierre Morel 
Reviewed-by: Halil Pasic 
Signed-off-by: Yi Min Zhao 
---
 hw/s390x/s390-pci-bus.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index bd8a3e1e1c..69f45e3715 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -397,6 +397,16 @@ static IOMMUTLBEntry 
s390_translate_iommu(IOMMUMemoryRegion *mr, hwaddr addr,
 return ret;
 }
 
+static void s390_pci_iommu_replay(IOMMUMemoryRegion *iommu,
+  IOMMUNotifier *notifier)
+{
+/* It's impossible to plug a pci device on s390x that already has iommu
+ * mappings which need to be replayed, that is due to the "one iommu per
+ * zpci device" construct. So we don't need iommu replay currently.
+ */
+return;
+}
+
 static S390PCIIOMMU *s390_pci_get_iommu(S390pciState *s, PCIBus *bus,
 int devfn)
 {
@@ -1045,6 +1055,7 @@ static void 
s390_iommu_memory_region_class_init(ObjectClass *klass, void *data)
 IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_CLASS(klass);
 
 imrc->translate = s390_translate_iommu;
+imrc->replay = s390_pci_iommu_replay;
 }
 
 static const TypeInfo s390_iommu_memory_region_info = {
-- 
2.11.0 (Apple Git-81)

[Qemu-devel] [PATCH v2 2/3] s390x/pci: fixup ind_offset of msix routing entry

[Yi Min Zhao]

The guest uses the mpcifc instruction to register the aibvo of a zpci
device, which is the starting offset of indicators in the indicator
area and thus remains constant. Each msix vector is an offset from the
aibvo. When we map a msix route to an adapter route, we should not
modify the starting offset, but instead add the vector to the starting
offset to get the absolute offset in the specific route.

Signed-off-by: Yi Min Zhao 
---
 target/s390x/kvm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 3d490c5e4b..21ce06966c 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2545,14 +2545,12 @@ int kvm_arch_fixup_msi_route(struct 
kvm_irq_routing_entry *route,
 return -ENODEV;
 }
 
-pbdev->routes.adapter.ind_offset = vec;
-
 route->type = KVM_IRQ_ROUTING_S390_ADAPTER;
 route->flags = 0;
 route->u.adapter.summary_addr = pbdev->routes.adapter.summary_addr;
 route->u.adapter.ind_addr = pbdev->routes.adapter.ind_addr;
 route->u.adapter.summary_offset = pbdev->routes.adapter.summary_offset;
-route->u.adapter.ind_offset = pbdev->routes.adapter.ind_offset;
+route->u.adapter.ind_offset = pbdev->routes.adapter.ind_offset + vec;
 route->u.adapter.adapter_id = pbdev->routes.adapter.adapter_id;
 return 0;
 }
-- 
2.11.0 (Apple Git-81)

[Qemu-devel] [PATCH v2 1/3] s390x/pci: remove idx from msix msg data

[Yi Min Zhao]

PCIDevice pointer has been a parameter of kvm_arch_fixup_msi_route().
So we don't need to store zpci idx in msix message data to find out the
specific zpci device. Instead, we could use pci device id to find its
corresponding zpci device.

Signed-off-by: Yi Min Zhao 
---
 hw/s390x/s390-pci-bus.c  | 16 +---
 hw/s390x/s390-pci-bus.h  |  2 ++
 hw/s390x/s390-pci-inst.c | 24 
 hw/s390x/s390-pci-stub.c |  6 ++
 target/s390x/kvm.c   |  7 +--
 5 files changed, 18 insertions(+), 37 deletions(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 0a31a4ae88..bd8a3e1e1c 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -199,8 +199,8 @@ static S390PCIBusDevice 
*s390_pci_find_dev_by_uid(S390pciState *s, uint16_t uid)
 return NULL;
 }
 
-static S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,
- const char *target)
+S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,
+  const char *target)
 {
 S390PCIBusDevice *pbdev;
 
@@ -465,19 +465,13 @@ static void s390_msi_ctrl_write(void *opaque, hwaddr 
addr, uint64_t data,
 unsigned int size)
 {
 S390PCIBusDevice *pbdev = opaque;
-uint32_t idx = data >> ZPCI_MSI_VEC_BITS;
 uint32_t vec = data & ZPCI_MSI_VEC_MASK;
 uint64_t ind_bit;
 uint32_t sum_bit;
-uint32_t e = 0;
 
-DPRINTF("write_msix data 0x%" PRIx64 " idx %d vec 0x%x\n", data, idx, vec);
-
-if (!pbdev) {
-e |= (vec << ERR_EVENT_MVN_OFFSET);
-s390_pci_generate_error_event(ERR_EVENT_NOMSI, idx, 0, addr, e);
-return;
-}
+assert(pbdev);
+DPRINTF("write_msix data 0x%" PRIx64 " idx %d vec 0x%x\n", data,
+pbdev->idx, vec);
 
 if (pbdev->state != ZPCI_FS_ENABLED) {
 return;
diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
index bd636abc28..560bd82a0f 100644
--- a/hw/s390x/s390-pci-bus.h
+++ b/hw/s390x/s390-pci-bus.h
@@ -322,6 +322,8 @@ void s390_pci_generate_error_event(uint16_t pec, uint32_t 
fh, uint32_t fid,
 S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx);
 S390PCIBusDevice *s390_pci_find_dev_by_fh(S390pciState *s, uint32_t fh);
 S390PCIBusDevice *s390_pci_find_dev_by_fid(S390pciState *s, uint32_t fid);
+S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,
+  const char *target);
 S390PCIBusDevice *s390_pci_find_next_avail_dev(S390pciState *s,
S390PCIBusDevice *pbdev);
 
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index eba9ffb5f2..8e088f3dc9 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -413,29 +413,6 @@ int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t 
r2)
 return 0;
 }
 
-static void update_msix_table_msg_data(S390PCIBusDevice *pbdev, uint64_t 
offset,
-   uint64_t *data, uint8_t len)
-{
-uint32_t val;
-uint8_t *msg_data;
-
-if (offset % PCI_MSIX_ENTRY_SIZE != 8) {
-return;
-}
-
-if (len != 4) {
-DPRINTF("access msix table msg data but len is %d\n", len);
-return;
-}
-
-msg_data = (uint8_t *)data - offset % PCI_MSIX_ENTRY_SIZE +
-   PCI_MSIX_ENTRY_VECTOR_CTRL;
-val = pci_get_long(msg_data) |
-((pbdev->fh & FH_MASK_INDEX) << ZPCI_MSI_VEC_BITS);
-pci_set_long(msg_data, val);
-DPRINTF("update msix msg_data to 0x%" PRIx64 "\n", *data);
-}
-
 static int trap_msix(S390PCIBusDevice *pbdev, uint64_t offset, uint8_t pcias)
 {
 if (pbdev->msix.available && pbdev->msix.table_bar == pcias &&
@@ -508,7 +485,6 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t 
r2)
 if (trap_msix(pbdev, offset, pcias)) {
 offset = offset - pbdev->msix.table_offset;
 mr = >pdev->msix_table_mmio;
-update_msix_table_msg_data(pbdev, offset, , len);
 } else {
 mr = pbdev->pdev->io_regions[pcias].memory;
 }
diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c
index 7a642d376c..e501e1b9ea 100644
--- a/hw/s390x/s390-pci-stub.c
+++ b/hw/s390x/s390-pci-stub.c
@@ -74,3 +74,9 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, 
uint32_t idx)
 {
 return NULL;
 }
+
+S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,
+  const char *target)
+{
+return NULL;
+}
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 1338c29528..3d490c5e4b 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2533,10 +2533,13 @@ int kvm_arch_fixup_msi_route(struct 
kvm_irq_routing_entry *route,
  uint64_t address, uint32_t data, PCIDevice *dev)
 {
 S390PCIBusDevice *pbdev;
-uint32_t idx = data >> 

[Qemu-devel] [PATCH v2 0/3] three zpci patches

[Yi Min Zhao]

This patch set contains three small zpci patches to fixup different issues.
1) remove zpci idx from msix message, instead we could use PCIDevice's id to
   find zpci device in kvm_arch_fixup_msi_route()
2) fixup ind_offset calculation for adapter interrupt routing entry
3) introduce our own iommu_replay callback

Yi Min Zhao (3):
  s390x/pci: remove idx from msix msg data
  s390x/pci: fixup ind_offset of msix routing entry
  s390x/pci: add iommu replay callback

 hw/s390x/s390-pci-bus.c  | 27 ---
 hw/s390x/s390-pci-bus.h  |  2 ++
 hw/s390x/s390-pci-inst.c | 24 
 hw/s390x/s390-pci-stub.c |  6 ++
 target/s390x/kvm.c   | 11 ++-
 5 files changed, 30 insertions(+), 40 deletions(-)

-- 
Change log:
from v1:
1) Add s390_pci_find_dev_by_target() in s390_pci_stub.c
2) Remove the accepted patch from the series (Thanks for Conny's help).
3) Fixup typo error.
4) Add more comment for s390_pci_iommu_replay().

[Qemu-devel] [PATCH v6 3/4] ACPI/unit-test: Add a new testcase for RAM allocation in numa node

[Dou Liyang]

As QEMU supports the memory-less node, it is possible that there is
no RAM in the first numa node(also be called as node0). eg:
  ... \
  -m 128,slots=3,maxmem=1G \
  -numa node -numa node,mem=128M \

But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
table. Only fixing it is not enough.

Add a testcase for this situation to make sure the ACPI table is
correct for guest.

Suggested-by: Eduardo Habkost 
Signed-off-by: Dou Liyang 
---
 tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 5104 bytes
 tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 224 bytes
 tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 7788 bytes
 tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 224 bytes
 tests/bios-tables-test.c  |  24 
 5 files changed, 24 insertions(+)
 create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
 create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
 create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
 create mode 100644 tests/acpi-test-data/q35/SRAT.numamem

diff --git a/tests/acpi-test-data/pc/DSDT.numamem 
b/tests/acpi-test-data/pc/DSDT.numamem
new file mode 100644
index 
..bc703d77ee48720af1b801c518b43511c95c852d
GIT binary patch
literal 5104
zcmb7IZExE~5}qX~n_5aXWizjC8^LKVXmL$b76k9SQ%DEh9oG8b|DMfK1
z(gvt&6i`^ERG;xE-sw(qx*XF!z}jjPX%ajXzq
zw)Zd3`{YZHwS3rmsXyOp`CsgNTR@obq*UN7;@fd>bkHW>7wH}lOw3;x+kz<+;?^`Xya)uvf@QYk*O7srT^929^Z
z(63x0Z^@d%=E?%?G|FWMJruoz0sP@PWP*8-tlj#VIsH94;*3Ze}dmP>U!{4qE
z3%vBYS9|OW!Z=at1=2Sc+mq77{0X)+xWdGVZ3hlYIS{M^y2LNdbhD6aE$wxA%i+Vt_OfTwcvY}08l4PJ2-Q=9}7b8sV#4=e3

[Qemu-devel] [PATCH v6 4/4] NUMA: Replace MAX_NODES with nb_numa_nodes in for loop

[Dou Liyang]

In QEMU, the number of the NUMA nodes is determined by parse_numa_opts().
Then, QEMU uses it for iteration, for example:
  for (i = 0; i < nb_numa_nodes; i++)

However, in memory_region_allocate_system_memory(), it uses MAX_NODES
not nb_numa_nodes.

So, replace MAX_NODES with nb_numa_nodes to keep code consistency and
reduce the loop times.

Signed-off-by: Dou Liyang 
Reviewed-by: Igor Mammedov 
Reviewed-by: Philippe Mathieu-Daudé 
---
 numa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/numa.c b/numa.c
index e32af04..5f2916d 100644
--- a/numa.c
+++ b/numa.c
@@ -567,7 +567,7 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, 
Object *owner,
 }
 
 memory_region_init(mr, owner, name, ram_size);
-for (i = 0; i < MAX_NODES; i++) {
+for (i = 0; i < nb_numa_nodes; i++) {
 uint64_t size = numa_info[i].node_mem;
 HostMemoryBackend *backend = numa_info[i].node_memdev;
 if (!backend) {
-- 
2.5.5

[Qemu-devel] [PATCH v6 0/4]ACPI: NUMA: Fix ACPI SRAT Memory Affinity building

[Dou Liyang]

v5 --> v6:
  - Split the unrelated code into a separate patch [2/4]

v4 --> v5:
  - Replace the original way with Eduardo's method.
  - Rewrite the testcase.
  - Drop the SLIT date
  - 2.11 develop tree is opened, So, Add the third patch for re-posting it. 

v3 --> v4:
  -add a new testcase.

This patchset fixs an ACPI building bug which caused by no RAM
in the first NUAM node. and also add a new testcase for the bug.

Dou Liyang (3):
  hw/acpi-build: Make assignment statement of next_base easy to read
  ACPI/unit-test: Add a new testcase for RAM allocation in numa node
  NUMA: Replace MAX_NODES with nb_numa_nodes in for loop

Eduardo Habkost (1):
  hw/acpi-build: Fix SRAT memory building in case of node 0 without RAM

 hw/i386/acpi-build.c  |  30 +++---
 numa.c|   2 +-
 tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 5104 bytes
 tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 224 bytes
 tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 7788 bytes
 tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 224 bytes
 tests/bios-tables-test.c  |  24 
 7 files changed, 48 insertions(+), 8 deletions(-)
 create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
 create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
 create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
 create mode 100644 tests/acpi-test-data/q35/SRAT.numamem

-- 
2.5.5

[Qemu-devel] [PATCH v6 1/4] hw/acpi-build: Fix SRAT memory building in case of node 0 without RAM

[Dou Liyang]

From: Eduardo Habkost 

Currently, Using the fisrt node without memory on the machine makes
QEMU unhappy. With this example command line:
  ... \
  -m 1024M,slots=4,maxmem=32G \
  -numa node,nodeid=0 \
  -numa node,mem=1024M,nodeid=1 \
  -numa node,nodeid=2 \
  -numa node,nodeid=3 \
Guest reports "No NUMA configuration found" and the NUMA topology is
wrong.

This is because when QEMU builds ACPI SRAT, it regards node 0 as the
default node to deal with the memory hole(640K-1M). this means the
node0 must have some memory(>1M), but, actually it can have no
memory.

Fix this problem by cut out the 640K hole in the same way the PCI
4G hole does.

Signed-off-by: Eduardo Habkost 
Signed-off-by: Dou Liyang 
---
 hw/i386/acpi-build.c | 28 ++--
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 98dd424..a0cf3bf 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2318,6 +2318,9 @@ build_tpm2(GArray *table_data, BIOSLinker *linker)
  (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL);
 }
 
+#define HOLE_640K_START  (640 * 1024)
+#define HOLE_640K_END   (1024 * 1024)
+
 static void
 build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
 {
@@ -2373,17 +2376,30 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
MachineState *machine)
 next_base = 0;
 numa_start = table_data->len;
 
-numamem = acpi_data_push(table_data, sizeof *numamem);
-build_srat_memory(numamem, 0, 640 * 1024, 0, MEM_AFFINITY_ENABLED);
-next_base = 1024 * 1024;
 for (i = 1; i < pcms->numa_nodes + 1; ++i) {
 mem_base = next_base;
 mem_len = pcms->node_mem[i - 1];
-if (i == 1) {
-mem_len -= 1024 * 1024;
-}
 next_base = mem_base + mem_len;
 
+/* Cut out the 640K hole */
+if (mem_base <= HOLE_640K_START &&
+next_base > HOLE_640K_START) {
+mem_len -= next_base - HOLE_640K_START;
+if (mem_len > 0) {
+numamem = acpi_data_push(table_data, sizeof *numamem);
+build_srat_memory(numamem, mem_base, mem_len, i - 1,
+  MEM_AFFINITY_ENABLED);
+}
+
+/* Check for the rare case: 640K < RAM < 1M */
+if (next_base <= HOLE_640K_END) {
+next_base = HOLE_640K_END;
+continue;
+}
+mem_base = HOLE_640K_END;
+mem_len = next_base - HOLE_640K_END;
+}
+
 /* Cut out the ACPI_PCI hole */
 if (mem_base <= pcms->below_4g_mem_size &&
 next_base > pcms->below_4g_mem_size) {
-- 
2.5.5

[Qemu-devel] [PATCH v6 2/4] hw/acpi-build: Make assignment statement of next_base easy to read

[Dou Liyang]

It may be hard to read the assignment statement of "next_base", so

S/next_base += (1ULL << 32) - pcms->below_4g_mem_size;
 /next_base = mem_base + mem_len;

... for readability.

No functionality change.

Signed-off-by: Dou Liyang 
---
 hw/i386/acpi-build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a0cf3bf..48525a1 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2411,7 +2411,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
MachineState *machine)
 }
 mem_base = 1ULL << 32;
 mem_len = next_base - pcms->below_4g_mem_size;
-next_base += (1ULL << 32) - pcms->below_4g_mem_size;
+next_base = mem_base + mem_len;
 }
 numamem = acpi_data_push(table_data, sizeof *numamem);
 build_srat_memory(numamem, mem_base, mem_len, i - 1,
-- 
2.5.5

Re: [Qemu-devel] [PATCH v5 1/3] hw/acpi-build: Fix SRAT memory building in case of node 0 without RAM

[Dou Liyang]

Hi, Eduardo

At 09/01/2017 05:36 AM, Eduardo Habkost wrote:

On Thu, Aug 31, 2017 at 08:04:26PM +0800, Dou Liyang wrote:

From: Eduardo Habkost 

Currently, Using the fisrt node without memory on the machine makes
QEMU unhappy. With this example command line:
  ... \
  -m 1024M,slots=4,maxmem=32G \
  -numa node,nodeid=0 \
  -numa node,mem=1024M,nodeid=1 \
  -numa node,nodeid=2 \
  -numa node,nodeid=3 \
Guest reports "No NUMA configuration found" and the NUMA topology is
wrong.

This is because when QEMU builds ACPI SRAT, it regards node 0 as the
default node to deal with the memory hole(640K-1M). this means the
node0 must have some memory(>1M), but, actually it can have no
memory.

Fix this problem by  cut out the 640K hole in the same way the PCI
4G hole does. Also do some cleanup.

Signed-off-by: Eduardo Habkost 
Signed-off-by: Dou Liyang 
---
 hw/i386/acpi-build.c | 30 +++---
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 98dd424..48525a1 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2318,6 +2318,9 @@ build_tpm2(GArray *table_data, BIOSLinker *linker)
  (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL);
 }

+#define HOLE_640K_START  (640 * 1024)
+#define HOLE_640K_END   (1024 * 1024)
+
 static void
 build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
 {
@@ -2373,17 +2376,30 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
MachineState *machine)
 next_base = 0;
 numa_start = table_data->len;

-numamem = acpi_data_push(table_data, sizeof *numamem);
-build_srat_memory(numamem, 0, 640 * 1024, 0, MEM_AFFINITY_ENABLED);
-next_base = 1024 * 1024;
 for (i = 1; i < pcms->numa_nodes + 1; ++i) {
 mem_base = next_base;
 mem_len = pcms->node_mem[i - 1];
-if (i == 1) {
-mem_len -= 1024 * 1024;
-}
 next_base = mem_base + mem_len;

+/* Cut out the 640K hole */
+if (mem_base <= HOLE_640K_START &&
+next_base > HOLE_640K_START) {
+mem_len -= next_base - HOLE_640K_START;
+if (mem_len > 0) {
+numamem = acpi_data_push(table_data, sizeof *numamem);
+build_srat_memory(numamem, mem_base, mem_len, i - 1,
+  MEM_AFFINITY_ENABLED);
+}
+
+/* Check for the rare case: 640K < RAM < 1M */
+if (next_base <= HOLE_640K_END) {
+next_base = HOLE_640K_END;
+continue;
+}
+mem_base = HOLE_640K_END;
+mem_len = next_base - HOLE_640K_END;
+}
+
 /* Cut out the ACPI_PCI hole */
 if (mem_base <= pcms->below_4g_mem_size &&
 next_base > pcms->below_4g_mem_size) {
@@ -2395,7 +2411,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
MachineState *machine)
 }
 mem_base = 1ULL << 32;
 mem_len = next_base - pcms->below_4g_mem_size;
-next_base += (1ULL << 32) - pcms->below_4g_mem_size;
+next_base = mem_base + mem_len;


Is this extra change intentional?



Yes, it is, Just for readability. :-)


I find the code more readable with it, but it should go in a
separate patch because it is unrelated to the bug fix.



Indeed, I will split it out.

Thanks,
dou.

Re: [Qemu-devel] [PATCH v19 0/2] virtio-crypto: virtio crypto device specification

[Longpeng (Mike)]

Ping...

Stefan, Halil, do you have any suggestion ?

-- 
Regards,
Longpeng(Mike)

On 2017/8/26 15:53, Longpeng(Mike) wrote:

> Hi guys,
> 
> I'll work on the virtio-crypto spec with Gonglei together, Because He is
> so busy on the inner production project.
> 
> ---
> v19 -> v18:
>  - fix some typos and grammar fixes [Stefan, Halil]
>  - rename VIRTIO_CRYPTO_F_STATELESS_MODE to VIRTIO_CRYPTO_F_MUX_MODE
>  - describe the VIRTIO_CRYPTO_STATUS in detial. [Halil]
>  - refactor and redescribe the controlq/dataq request's format
>of mux mode. [Halil]
>  - other small fixes. [Halil]
> 
> v18 -> v17:
>  - fix many English grammar problems suggested by Stefan, Thanks a lot!
> 
> v17 -> v16:
>  - Some grammar fixes [Stefan, Halil, Michael]
>  - add a section named "Supported crypto services" in order to explain bit
>numbers and valuse clearly. [Halil, Cornelia]
>  - avoid word reptition [Halil]
>  - rename non-session mode to stateless mode [Halil]
>  - change descriptions for all elements in struct virtio_crypto_config [Halil]
>  - add Halil as a reviewer in the ackonwledgement part, thanks for his work.
>  - other fixes here and there.
> 
> Changes since v15:
>  - use feature bits for non-session mode in order to keep compatibility with
>pre-existing code. [Halil & Michael]
>  - introduce VIRTIO_CRYPTO_F_ NON_SESSION_MODE feature bit to control all 
> other
>non-session mode feature bits.
>  - fix some typos. [Stefan]
>  - introduce struct virtio_crypto_op_data_req_mux to support both session
>and non-session based crypto operations and keep compatibility with
>pre-existing code.
> 
> Changes since v14:
>  - drop VIRTIO_CRYPTO_S_STARTED status [Halil & Cornelia]
>  - correct a sentence about dataqueue and controlq in the first paragraph. 
> [Halil]
>  - change a MAY to MUST about max_dataqueues. [Halil]
>  - add non-session mode support
>a) add four features for different crypto services to identify wheather 
> support session mode.
>b) rewrite some
> 
> For pervious versions of virtio crypto spec, Pls see:
> 
> [v18]:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg444897.html
> 
> [v14]:
> https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg02212.html
> 
> [v13]:
> https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg07348.html
> 
> For more information, please see:
>  http://qemu-project.org/Features/VirtioCrypto
> 
> ---
> Gonglei (2):
>   virtio-crypto: Add virtio crypto device specification
>   virtio-crypto: Add conformance clauses
> 
>  acknowledgements.tex |3 +
>  conformance.tex  |   29 +
>  content.tex  |2 +
>  virtio-crypto.tex| 1470 
> ++
>  4 files changed, 1504 insertions(+)
>  create mode 100644 virtio-crypto.tex
> 


-- 
Regards,
Longpeng(Mike)

Re: [Qemu-devel] [PATCH v3 5/9] IDE: replace DEBUG_AIO with trace events

[John Snow]

On 08/31/2017 08:27 PM, Philippe Mathieu-Daudé wrote:
> On 08/31/2017 09:14 PM, John Snow wrote:
>> Signed-off-by: John Snow 
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> 

TY, and btw the reasoning behind what I went with:

Since s->dma_cmd is itself a type `enum ide_dma_cmd` then by definition
s->dma_cmd (even if corrupted or wrong) must fit inside the width of
that type. Performing the range checking in the getter is adequate for
protecting the table in this case, if I understand Laszlo's rationales
correctly.

In case case of the AHCI table, I do not use a getter since the IRQBIT
property is passed in statically for each instance and shouldn't have a
chance to get out-of-spec. (If it does, something MASSIVELY bad has
corrupted everything, and then we've got worse problems.)

Thanks,
John

Re: [Qemu-devel] [PATCH v3 0/9] IDE: replace printfs with tracing

[John Snow]

On 08/31/2017 08:27 PM, no-re...@patchew.org wrote:
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Message-id: 20170901001502.29915-1-js...@redhat.com
> Subject: [Qemu-devel] [PATCH v3 0/9] IDE: replace printfs with tracing
> Type: series
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> 
> BASE=base
> n=1
> total=$(git log --oneline $BASE.. | wc -l)
> failed=0
> 
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> 
> commits="$(git log --format=%H --reverse $BASE..)"
> for c in $commits; do
> echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; 
> then
> failed=1
> echo
> fi
> n=$((n+1))
> done
> 
> exit $failed
> === TEST SCRIPT END ===
> 
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
>  t [tag update]
> patchew/1503938283-12404-1-git-send-email-sundeep.l...@gmail.com -> 
> patchew/1503938283-12404-1-git-send-email-sundeep.l...@gmail.com
>  t [tag update]
> patchew/1504190408-11143-1-git-send-email-th...@redhat.com -> 
> patchew/1504190408-11143-1-git-send-email-th...@redhat.com
>  * [new tag]   patchew/20170901001502.29915-1-js...@redhat.com -> 
> patchew/20170901001502.29915-1-js...@redhat.com
> Switched to a new branch 'test'
> b45bc7135d AHCI: remove DPRINTF macro
> 52a114ea94 AHCI: pretty-print FIS to buffer instead of stderr
> dc1fb9df07 AHCI: Rework IRQ constants
> 01eb2629b7 AHCI: Replace DPRINTF with trace-events
> b64224c313 IDE: replace DEBUG_AIO with trace events
> 0a54222e15 ATAPI: Replace DEBUG_IDE_ATAPI with tracing events
> 1c976d454b IDE: add tracing for data ports
> b46b3c2141 IDE: Add register hints to tracing
> 1da9fe67ec IDE: replace DEBUG_IDE with tracing system
> 
> === OUTPUT BEGIN ===
> Checking PATCH 1/9: IDE: replace DEBUG_IDE with tracing system...
> ERROR: spaces required around that '|' (ctx:VxV)
> #143: FILE: hw/ide/core.c:1197:
> +if (reg_num != 7 && (s->status & (BUSY_STAT|DRQ_STAT))) {

Existing damage, but I suppose I'll fix it since I'm here.

> ^
> 
> total: 1 errors, 0 warnings, 337 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> Checking PATCH 2/9: IDE: Add register hints to tracing...
> Checking PATCH 3/9: IDE: add tracing for data ports...
> Checking PATCH 4/9: ATAPI: Replace DEBUG_IDE_ATAPI with tracing events...
> Checking PATCH 5/9: IDE: replace DEBUG_AIO with trace events...
> Checking PATCH 6/9: AHCI: Replace DPRINTF with trace-events...
> ERROR: Hex numbers must be prefixed with '0x'
> #544: FILE: hw/ide/trace-events:91:
> +handle_reg_h2d_fis_pmp(void *s, int port, char b0, char b1, char b2) 
> "ahci(%p)[%d]: Port Multiplier not supported, FIS: 0x%02x-%02x-%02x"
> 

False positive, it's part of a hex sequence so the context is obvious.

> ERROR: Hex numbers must be prefixed with '0x'
> #545: FILE: hw/ide/trace-events:92:
> +handle_reg_h2d_fis_res(void *s, int port, char b0, char b1, char b2) 
> "ahci(%p)[%d]: Reserved flags set in H2D Register FIS, FIS: 0x%02x-%02x-%02x"
> 

Same

> ERROR: Hex numbers must be prefixed with '0x'
> #551: FILE: hw/ide/trace-events:98:
> +handle_cmd_unhandled_fis(void *s, int port, uint8_t b0, uint8_t b1, uint8_t 
> b2) "ahci(%p)[%d]: unhandled FIS type. cmd_fis: 0x%02x-%02x-%02x"
> 

Same

> total: 3 errors, 0 warnings, 496 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> Checking PATCH 7/9: AHCI: Rework IRQ constants...
> Checking PATCH 8/9: AHCI: pretty-print FIS to buffer instead of stderr...
> WARNING: line over 80 characters
> #52: FILE: hw/ide/ahci.c:1206:
> +char *pretty_fis = ahci_pretty_buffer_fis(ide_state->io_buffer, 
> 0x10);
> 

Will adjust.

> total: 0 errors, 1 warnings, 60 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> Checking PATCH 9/9: AHCI: remove DPRINTF macro...
> === OUTPUT END ===
> 
> Test command exited with code: 1
> 
> 
> ---
> Email generated automatically by Patchew [http://patchew.org/].
> Please send your feedback to patchew-de...@freelists.org
> 

Re: [Qemu-devel] [PATCH v3 5/9] IDE: replace DEBUG_AIO with trace events

[Philippe Mathieu-Daudé]

On 08/31/2017 09:14 PM, John Snow wrote:

Signed-off-by: John Snow 


Reviewed-by: Philippe Mathieu-Daudé 


---
  hw/ide/atapi.c|  5 +
  hw/ide/core.c | 24 +---
  hw/ide/trace-events   |  3 +++
  include/hw/ide/internal.h |  6 --
  4 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 9b84a1b..c0509c8 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -416,10 +416,7 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int 
ret)
  s->io_buffer_size = n * 2048;
  data_offset = 0;
  }
-#ifdef DEBUG_AIO
-printf("aio_read_cd: lba=%u n=%d\n", s->lba, n);
-#endif
-
+trace_ide_atapi_cmd_read_dma_cb_aio(s, s->lba, n);
  s->bus->dma->iov.iov_base = (void *)(s->io_buffer + data_offset);
  s->bus->dma->iov.iov_len = n * ATAPI_SECTOR_SIZE;
  qemu_iovec_init_external(>bus->dma->qiov, >bus->dma->iov, 1);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 82a19b1..2cc2a08 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -58,6 +58,21 @@ static const int smart_attributes[][12] = {
  { 190,  0x03, 0x00, 0x45, 0x45, 0x1f, 0x00, 0x1f, 0x1f, 0x00, 0x00, 0x32},
  };
  
+const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT] = {

+[IDE_DMA_READ] = "DMA READ",
+[IDE_DMA_WRITE] = "DMA WRITE",
+[IDE_DMA_TRIM] = "DMA TRIM",
+[IDE_DMA_ATAPI] = "DMA ATAPI"
+};
+
+static const char *IDE_DMA_CMD_str(enum ide_dma_cmd enval)
+{
+if (enval >= 0 && enval < IDE_DMA__COUNT) {
+return IDE_DMA_CMD_lookup[enval];
+}
+return "DMA UNKNOWN CMD";
+}
+
  static void ide_dummy_transfer_stop(IDEState *s);
  
  static void padstr(char *str, const char *src, int len)

@@ -860,10 +875,7 @@ static void ide_dma_cb(void *opaque, int ret)
  goto eot;
  }
  
-#ifdef DEBUG_AIO

-printf("ide_dma_cb: sector_num=%" PRId64 " n=%d, cmd_cmd=%d\n",
-   sector_num, n, s->dma_cmd);
-#endif
+trace_ide_dma_cb(s, sector_num, n, IDE_DMA_CMD_str(s->dma_cmd));
  
  if ((s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) &&

  !ide_sect_range_ok(s, sector_num, n)) {
@@ -2391,9 +2403,7 @@ void ide_bus_reset(IDEBus *bus)
  
  /* pending async DMA */

  if (bus->dma->aiocb) {
-#ifdef DEBUG_AIO
-printf("aio_cancel\n");
-#endif
+trace_ide_bus_reset_aio();
  blk_aio_cancel(bus->dma->aiocb);
  bus->dma->aiocb = NULL;
  }
diff --git a/hw/ide/trace-events b/hw/ide/trace-events
index 8c79a6c..cc8949c 100644
--- a/hw/ide/trace-events
+++ b/hw/ide/trace-events
@@ -18,6 +18,8 @@ ide_cancel_dma_sync_remaining(void) "draining all remaining 
requests"
  ide_sector_read(int64_t sector_num, int nsectors) "sector=%"PRId64" 
nsectors=%d"
  ide_sector_write(int64_t sector_num, int nsectors) "sector=%"PRId64" 
nsectors=%d"
  ide_reset(void *s) "IDEstate %p"
+ide_bus_reset_aio(void) "aio_cancel"
+ide_dma_cb(void *s, int64_t sector_num, int n, const char *dma) "IDEState %p; 
sector_num=%"PRId64" n=%d cmd=%s"
  
  # BMDMA HBAs:
  
@@ -51,5 +53,6 @@ ide_atapi_cmd_reply_end_new(void *s, int status) "IDEState: %p; new transfer sta

  ide_atapi_cmd_check_status(void *s) "IDEState: %p"
  ide_atapi_cmd_read(void *s, const char *method, int lba, int nb_sectors) 
"IDEState: %p; read %s: LBA=%d nb_sectors=%d"
  ide_atapi_cmd(void *s, uint8_t cmd) "IDEState: %p; cmd: 0x%02x"
+ide_atapi_cmd_read_dma_cb_aio(void *s, int lba, int n) "IDEState: %p; aio read: 
lba=%d n=%d"
  # Warning: Verbose
  ide_atapi_cmd_packet(void *s, uint16_t limit, const char *packet) "IDEState: %p; 
limit=0x%x packet: %s"
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 74efe8a..db9fde0 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -14,7 +14,6 @@
  #include "block/scsi.h"
  
  /* debug IDE devices */

-//#define DEBUG_AIO
  #define USE_DMA_CDROM
  
  typedef struct IDEBus IDEBus;

@@ -333,12 +332,15 @@ struct unreported_events {
  };
  
  enum ide_dma_cmd {

-IDE_DMA_READ,
+IDE_DMA_READ = 0,
  IDE_DMA_WRITE,
  IDE_DMA_TRIM,
  IDE_DMA_ATAPI,
+IDE_DMA__COUNT
  };
  
+extern const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT];

+
  #define ide_cmd_is_read(s) \
((s)->dma_cmd == IDE_DMA_READ)
  

Re: [Qemu-devel] [PATCH v3 0/9] IDE: replace printfs with tracing

[no-reply]

Hi,

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

Message-id: 20170901001502.29915-1-js...@redhat.com
Subject: [Qemu-devel] [PATCH v3 0/9] IDE: replace printfs with tracing
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]
patchew/1503938283-12404-1-git-send-email-sundeep.l...@gmail.com -> 
patchew/1503938283-12404-1-git-send-email-sundeep.l...@gmail.com
 t [tag update]
patchew/1504190408-11143-1-git-send-email-th...@redhat.com -> 
patchew/1504190408-11143-1-git-send-email-th...@redhat.com
 * [new tag]   patchew/20170901001502.29915-1-js...@redhat.com -> 
patchew/20170901001502.29915-1-js...@redhat.com
Switched to a new branch 'test'
b45bc7135d AHCI: remove DPRINTF macro
52a114ea94 AHCI: pretty-print FIS to buffer instead of stderr
dc1fb9df07 AHCI: Rework IRQ constants
01eb2629b7 AHCI: Replace DPRINTF with trace-events
b64224c313 IDE: replace DEBUG_AIO with trace events
0a54222e15 ATAPI: Replace DEBUG_IDE_ATAPI with tracing events
1c976d454b IDE: add tracing for data ports
b46b3c2141 IDE: Add register hints to tracing
1da9fe67ec IDE: replace DEBUG_IDE with tracing system

=== OUTPUT BEGIN ===
Checking PATCH 1/9: IDE: replace DEBUG_IDE with tracing system...
ERROR: spaces required around that '|' (ctx:VxV)
#143: FILE: hw/ide/core.c:1197:
+if (reg_num != 7 && (s->status & (BUSY_STAT|DRQ_STAT))) {
^

total: 1 errors, 0 warnings, 337 lines checked

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

Checking PATCH 2/9: IDE: Add register hints to tracing...
Checking PATCH 3/9: IDE: add tracing for data ports...
Checking PATCH 4/9: ATAPI: Replace DEBUG_IDE_ATAPI with tracing events...
Checking PATCH 5/9: IDE: replace DEBUG_AIO with trace events...
Checking PATCH 6/9: AHCI: Replace DPRINTF with trace-events...
ERROR: Hex numbers must be prefixed with '0x'
#544: FILE: hw/ide/trace-events:91:
+handle_reg_h2d_fis_pmp(void *s, int port, char b0, char b1, char b2) 
"ahci(%p)[%d]: Port Multiplier not supported, FIS: 0x%02x-%02x-%02x"

ERROR: Hex numbers must be prefixed with '0x'
#545: FILE: hw/ide/trace-events:92:
+handle_reg_h2d_fis_res(void *s, int port, char b0, char b1, char b2) 
"ahci(%p)[%d]: Reserved flags set in H2D Register FIS, FIS: 0x%02x-%02x-%02x"

ERROR: Hex numbers must be prefixed with '0x'
#551: FILE: hw/ide/trace-events:98:
+handle_cmd_unhandled_fis(void *s, int port, uint8_t b0, uint8_t b1, uint8_t 
b2) "ahci(%p)[%d]: unhandled FIS type. cmd_fis: 0x%02x-%02x-%02x"

total: 3 errors, 0 warnings, 496 lines checked

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

Checking PATCH 7/9: AHCI: Rework IRQ constants...
Checking PATCH 8/9: AHCI: pretty-print FIS to buffer instead of stderr...
WARNING: line over 80 characters
#52: FILE: hw/ide/ahci.c:1206:
+char *pretty_fis = ahci_pretty_buffer_fis(ide_state->io_buffer, 
0x10);

total: 0 errors, 1 warnings, 60 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 9/9: AHCI: remove DPRINTF macro...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

[Qemu-devel] [PATCH v3 8/9] AHCI: pretty-print FIS to buffer instead of stderr

[John Snow]

The current FIS printing routines dump the FIS to screen. adjust this
such that it dumps to buffer instead, then use this ability to have
FIS dump mechanisms via trace-events instead of compiled defines.

Signed-off-by: John Snow 
---
 hw/ide/ahci.c   | 28 ++--
 hw/ide/trace-events |  4 
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 82cc196..6c98047 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -644,20 +644,21 @@ static void ahci_reset_port(AHCIState *s, int port)
 ahci_init_d2h(d);
 }
 
-static void debug_print_fis(uint8_t *fis, int cmd_len)
+/* Buffer pretty output based on a raw FIS structure. */
+static char *ahci_pretty_buffer_fis(uint8_t *fis, int cmd_len)
 {
-#if DEBUG_AHCI
 int i;
+GString *s = g_string_new("FIS:");
 
-fprintf(stderr, "fis:");
 for (i = 0; i < cmd_len; i++) {
 if ((i & 0xf) == 0) {
-fprintf(stderr, "\n%02x:",i);
+g_string_append_printf(s, "\n0x%02x: ", i);
 }
-fprintf(stderr, "%02x ",fis[i]);
+g_string_append_printf(s, "%02x ", fis[i]);
 }
-fprintf(stderr, "\n");
-#endif
+g_string_append_c(s, '\n');
+
+return g_string_free(s, FALSE);
 }
 
 static bool ahci_map_fis_address(AHCIDevice *ad)
@@ -1201,7 +1202,11 @@ static void handle_reg_h2d_fis(AHCIState *s, int port,
  * table to ide_state->io_buffer */
 if (opts & AHCI_CMD_ATAPI) {
 memcpy(ide_state->io_buffer, _fis[AHCI_COMMAND_TABLE_ACMD], 0x10);
-debug_print_fis(ide_state->io_buffer, 0x10);
+if (trace_event_get_state_backends(TRACE_HANDLE_REG_H2D_FIS_DUMP)) {
+char *pretty_fis = ahci_pretty_buffer_fis(ide_state->io_buffer, 
0x10);
+trace_handle_reg_h2d_fis_dump(s, port, pretty_fis);
+g_free(pretty_fis);
+}
 s->dev[port].done_atapi_packet = false;
 /* XXX send PIO setup FIS */
 }
@@ -1256,8 +1261,11 @@ static int handle_cmd(AHCIState *s, int port, uint8_t 
slot)
 trace_handle_cmd_badmap(s, port, cmd_len);
 goto out;
 }
-debug_print_fis(cmd_fis, 0x80);
-
+if (trace_event_get_state_backends(TRACE_HANDLE_CMD_FIS_DUMP)) {
+char *pretty_fis = ahci_pretty_buffer_fis(cmd_fis, 0x80);
+trace_handle_cmd_fis_dump(s, port, pretty_fis);
+g_free(pretty_fis);
+}
 switch (cmd_fis[0]) {
 case SATA_FIS_TYPE_REGISTER_H2D:
 handle_reg_h2d_fis(s, port, slot, cmd_fis);
diff --git a/hw/ide/trace-events b/hw/ide/trace-events
index e15fd77..601bd97 100644
--- a/hw/ide/trace-events
+++ b/hw/ide/trace-events
@@ -105,3 +105,7 @@ ahci_cmd_done(void *s, int port) "ahci(%p)[%d]: cmd done"
 ahci_reset(void *s) "ahci(%p): HBA reset"
 allwinner_ahci_mem_read(void *s, void *a, uint64_t addr, uint64_t val, 
unsigned size) "ahci(%p): read a=%p addr=0x%"HWADDR_PRIx" val=0x%"PRIx64", 
size=%d"
 allwinner_ahci_mem_write(void *s, void *a, uint64_t addr, uint64_t val, 
unsigned size) "ahci(%p): write a=%p addr=0x%"HWADDR_PRIx" val=0x%"PRIx64", 
size=%d"
+
+# Warning: Verbose
+handle_reg_h2d_fis_dump(void *s, int port, const char *fis) "ahci(%p)[%d]: %s"
+handle_cmd_fis_dump(void *s, int port, const char *fis) "ahci(%p)[%d]: %s"
-- 
2.9.5

[Qemu-devel] [PATCH v3 9/9] AHCI: remove DPRINTF macro

[John Snow]

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: John Snow 
---
 hw/ide/ahci.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 6c98047..14b71a3 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -34,17 +34,8 @@
 #include "hw/ide/pci.h"
 #include "hw/ide/ahci_internal.h"
 
-#define DEBUG_AHCI 0
 #include "trace.h"
 
-#define DPRINTF(port, fmt, ...) \
-do { \
-if (DEBUG_AHCI) { \
-fprintf(stderr, "ahci: %s: [%d] ", __func__, port); \
-fprintf(stderr, fmt, ## __VA_ARGS__); \
-} \
-} while (0)
-
 static void check_cmd(AHCIState *s, int port);
 static int handle_cmd(AHCIState *s, int port, uint8_t slot);
 static void ahci_reset_port(AHCIState *s, int port);
-- 
2.9.5

[Qemu-devel] [PATCH v3 6/9] AHCI: Replace DPRINTF with trace-events

[John Snow]

There are a few hangers-on that will be dealt with individually
in forthcoming patches.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: John Snow 
---
 hw/ide/ahci.c   | 157 +++-
 hw/ide/trace-events |  49 
 2 files changed, 117 insertions(+), 89 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 406a1b5..c60a000 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -35,6 +35,7 @@
 #include "hw/ide/ahci_internal.h"
 
 #define DEBUG_AHCI 0
+#include "trace.h"
 
 #define DPRINTF(port, fmt, ...) \
 do { \
@@ -114,9 +115,9 @@ static uint32_t  ahci_port_read(AHCIState *s, int port, int 
offset)
 default:
 val = 0;
 }
-DPRINTF(port, "offset: 0x%x val: 0x%x\n", offset, val);
-return val;
 
+trace_ahci_port_read(s, port, offset, val);
+return val;
 }
 
 static void ahci_irq_raise(AHCIState *s, AHCIDevice *dev)
@@ -125,7 +126,7 @@ static void ahci_irq_raise(AHCIState *s, AHCIDevice *dev)
 PCIDevice *pci_dev = (PCIDevice *) object_dynamic_cast(OBJECT(dev_state),
TYPE_PCI_DEVICE);
 
-DPRINTF(0, "raise irq\n");
+trace_ahci_irq_raise(s);
 
 if (pci_dev && msi_enabled(pci_dev)) {
 msi_notify(pci_dev, 0);
@@ -140,7 +141,7 @@ static void ahci_irq_lower(AHCIState *s, AHCIDevice *dev)
 PCIDevice *pci_dev = (PCIDevice *) object_dynamic_cast(OBJECT(dev_state),
TYPE_PCI_DEVICE);
 
-DPRINTF(0, "lower irq\n");
+trace_ahci_irq_lower(s);
 
 if (!pci_dev || !msi_enabled(pci_dev)) {
 qemu_irq_lower(s->irq);
@@ -150,8 +151,7 @@ static void ahci_irq_lower(AHCIState *s, AHCIDevice *dev)
 static void ahci_check_irq(AHCIState *s)
 {
 int i;
-
-DPRINTF(-1, "check irq %#x\n", s->control_regs.irqstatus);
+uint32_t old_irq = s->control_regs.irqstatus;
 
 s->control_regs.irqstatus = 0;
 for (i = 0; i < s->ports; i++) {
@@ -160,7 +160,7 @@ static void ahci_check_irq(AHCIState *s)
 s->control_regs.irqstatus |= (1 << i);
 }
 }
-
+trace_ahci_check_irq(s, old_irq, s->control_regs.irqstatus);
 if (s->control_regs.irqstatus &&
 (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
 ahci_irq_raise(s, NULL);
@@ -240,7 +240,7 @@ static void  ahci_port_write(AHCIState *s, int port, int 
offset, uint32_t val)
 {
 AHCIPortRegs *pr = >dev[port].port_regs;
 
-DPRINTF(port, "offset: 0x%x val: 0x%x\n", offset, val);
+trace_ahci_port_write(s, port, offset, val);
 switch (offset) {
 case PORT_LST_ADDR:
 pr->lst_addr = val;
@@ -341,8 +341,6 @@ static uint64_t ahci_mem_read_32(void *opaque, hwaddr addr)
 val = s->control_regs.version;
 break;
 }
-
-DPRINTF(-1, "(addr 0x%08X), val 0x%08X\n", (unsigned) addr, val);
 } else if ((addr >= AHCI_PORT_REGS_START_ADDR) &&
(addr < (AHCI_PORT_REGS_START_ADDR +
 (s->ports * AHCI_PORT_ADDR_OFFSET_LEN {
@@ -350,6 +348,7 @@ static uint64_t ahci_mem_read_32(void *opaque, hwaddr addr)
  addr & AHCI_PORT_ADDR_OFFSET_MASK);
 }
 
+trace_ahci_mem_read_32(s, addr, val);
 return val;
 }
 
@@ -379,8 +378,7 @@ static uint64_t ahci_mem_read(void *opaque, hwaddr addr, 
unsigned size)
 val = (hi << 32 | lo) >> (ofst * 8);
 }
 
-DPRINTF(-1, "addr=0x%" HWADDR_PRIx " val=0x%" PRIx64 ", size=%d\n",
-addr, val, size);
+trace_ahci_mem_read(opaque, size, addr, val);
 return val;
 }
 
@@ -390,8 +388,7 @@ static void ahci_mem_write(void *opaque, hwaddr addr,
 {
 AHCIState *s = opaque;
 
-DPRINTF(-1, "addr=0x%" HWADDR_PRIx " val=0x%" PRIx64 ", size=%d\n",
-addr, val, size);
+trace_ahci_mem_write(s, size, addr, val);
 
 /* Only aligned reads are allowed on AHCI */
 if (addr & 3) {
@@ -401,15 +398,12 @@ static void ahci_mem_write(void *opaque, hwaddr addr,
 }
 
 if (addr < AHCI_GENERIC_HOST_CONTROL_REGS_MAX_ADDR) {
-DPRINTF(-1, "(addr 0x%08X), val 0x%08"PRIX64"\n", (unsigned) addr, 
val);
-
 switch (addr) {
 case HOST_CAP: /* R/WO, RO */
 /* FIXME handle R/WO */
 break;
 case HOST_CTL: /* R/W */
 if (val & HOST_CTL_RESET) {
-DPRINTF(-1, "HBA Reset\n");
 ahci_reset(s);
 } else {
 s->control_regs.ghc = (val & 0x3) | HOST_CTL_AHCI_EN;
@@ -427,7 +421,7 @@ static void ahci_mem_write(void *opaque, hwaddr addr,
 /* FIXME report write? */
 break;
 default:
-DPRINTF(-1, "write to unknown register 0x%x\n", 
(unsigned)addr);
+trace_ahci_mem_write_unknown(s, size, addr, val);
 }
 } else if ((addr >= 

[Qemu-devel] [PATCH v3 7/9] AHCI: Rework IRQ constants

[John Snow]

Create a new enum so that we can name the IRQ bits, which will make debugging
them a little nicer if we can print them out. Not handled in this patch, but
this will make it possible to get a nice debug printf detailing exactly which
status bits are set, as it can be multiple at any given time.

As a consequence of this patch, it is no longer possible to set multiple IRQ
codes at once, but nothing was utilizing this ability anyway.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: John Snow 
---
 hw/ide/ahci.c  | 49 ++---
 hw/ide/ahci_internal.h | 44 +++-
 hw/ide/trace-events|  2 +-
 3 files changed, 74 insertions(+), 21 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index c60a000..82cc196 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -56,6 +56,27 @@ static bool ahci_map_fis_address(AHCIDevice *ad);
 static void ahci_unmap_clb_address(AHCIDevice *ad);
 static void ahci_unmap_fis_address(AHCIDevice *ad);
 
+static const char *AHCIPortIRQ_lookup[AHCI_PORT_IRQ__COUNT] = {
+[AHCI_PORT_IRQ_BIT_DHRS] = "DHRS",
+[AHCI_PORT_IRQ_BIT_PSS]  = "PSS",
+[AHCI_PORT_IRQ_BIT_DSS]  = "DSS",
+[AHCI_PORT_IRQ_BIT_SDBS] = "SDBS",
+[AHCI_PORT_IRQ_BIT_UFS]  = "UFS",
+[AHCI_PORT_IRQ_BIT_DPS]  = "DPS",
+[AHCI_PORT_IRQ_BIT_PCS]  = "PCS",
+[AHCI_PORT_IRQ_BIT_DMPS] = "DMPS",
+[8 ... 21]   = "RESERVED",
+[AHCI_PORT_IRQ_BIT_PRCS] = "PRCS",
+[AHCI_PORT_IRQ_BIT_IPMS] = "IPMS",
+[AHCI_PORT_IRQ_BIT_OFS]  = "OFS",
+[25] = "RESERVED",
+[AHCI_PORT_IRQ_BIT_INFS] = "INFS",
+[AHCI_PORT_IRQ_BIT_IFS]  = "IFS",
+[AHCI_PORT_IRQ_BIT_HBDS] = "HBDS",
+[AHCI_PORT_IRQ_BIT_HBFS] = "HBFS",
+[AHCI_PORT_IRQ_BIT_TFES] = "TFES",
+[AHCI_PORT_IRQ_BIT_CPDS] = "CPDS"
+};
 
 static uint32_t  ahci_port_read(AHCIState *s, int port, int offset)
 {
@@ -170,12 +191,18 @@ static void ahci_check_irq(AHCIState *s)
 }
 
 static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d,
- int irq_type)
+ enum AHCIPortIRQ irqbit)
 {
-DPRINTF(d->port_no, "trigger irq %#x -> %x\n",
-irq_type, d->port_regs.irq_mask & irq_type);
+g_assert(irqbit >= 0 && irqbit < 32);
+uint32_t irq = 1U << irqbit;
+uint32_t irqstat = d->port_regs.irq_stat | irq;
 
-d->port_regs.irq_stat |= irq_type;
+trace_ahci_trigger_irq(s, d->port_no,
+   AHCIPortIRQ_lookup[irqbit], irq,
+   d->port_regs.irq_stat, irqstat,
+   irqstat & d->port_regs.irq_mask);
+
+d->port_regs.irq_stat = irqstat;
 ahci_check_irq(s);
 }
 
@@ -718,7 +745,7 @@ static void ahci_write_fis_sdb(AHCIState *s, 
NCQTransferState *ncq_tfs)
 
 /* Trigger IRQ if interrupt bit is set (which currently, it always is) */
 if (sdb_fis->flags & 0x40) {
-ahci_trigger_irq(s, ad, PORT_IRQ_SDB_FIS);
+ahci_trigger_irq(s, ad, AHCI_PORT_IRQ_BIT_SDBS);
 }
 }
 
@@ -761,10 +788,10 @@ static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t 
len)
 ad->port.ifs[0].status;
 
 if (pio_fis[2] & ERR_STAT) {
-ahci_trigger_irq(ad->hba, ad, PORT_IRQ_TF_ERR);
+ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_TFES);
 }
 
-ahci_trigger_irq(ad->hba, ad, PORT_IRQ_PIOS_FIS);
+ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_PSS);
 }
 
 static bool ahci_write_fis_d2h(AHCIDevice *ad)
@@ -804,10 +831,10 @@ static bool ahci_write_fis_d2h(AHCIDevice *ad)
 ad->port.ifs[0].status;
 
 if (d2h_fis[2] & ERR_STAT) {
-ahci_trigger_irq(ad->hba, ad, PORT_IRQ_TF_ERR);
+ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_TFES);
 }
 
-ahci_trigger_irq(ad->hba, ad, PORT_IRQ_D2H_REG_FIS);
+ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_DHRS);
 return true;
 }
 
@@ -1082,7 +1109,7 @@ static void process_ncq_command(AHCIState *s, int port, 
uint8_t *cmd_fis,
  "is smaller than the requested size (0x%zx)",
  ncq_tfs->sglist.size, size);
 ncq_err(ncq_tfs);
-ahci_trigger_irq(ad->hba, ad, PORT_IRQ_OVERFLOW);
+ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_OFS);
 return;
 } else if (ncq_tfs->sglist.size != size) {
 trace_process_ncq_command_large(s, port, tag,
@@ -1225,7 +1252,7 @@ static int handle_cmd(AHCIState *s, int port, uint8_t 
slot)
 trace_handle_cmd_badfis(s, port);
 return -1;
 } else if (cmd_len != 0x80) {
-ahci_trigger_irq(s, >dev[port], PORT_IRQ_HBUS_ERR);
+ahci_trigger_irq(s, >dev[port], AHCI_PORT_IRQ_BIT_HBFS);
 trace_handle_cmd_badmap(s, port, cmd_len);
 goto out;
 }
diff --git a/hw/ide/ahci_internal.h b/hw/ide/ahci_internal.h
index 1e21169..ce2e818 100644
--- a/hw/ide/ahci_internal.h
+++ b/hw/ide/ahci_internal.h
@@ -91,6 

[Qemu-devel] [PATCH v3 4/9] ATAPI: Replace DEBUG_IDE_ATAPI with tracing events

[John Snow]

As part of the ongoing effort to modernize the tracing facilities for
the IDE family of devices, remove PRINTFs in the ATAPI device with
actual tracing events.

Signed-off-by: John Snow 
---
 hw/ide/atapi.c| 64 +--
 hw/ide/trace-events   | 15 +++
 include/hw/ide/internal.h |  1 -
 3 files changed, 38 insertions(+), 42 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index fc1d19c..9b84a1b 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -27,6 +27,7 @@
 #include "hw/ide/internal.h"
 #include "hw/scsi/scsi.h"
 #include "sysemu/block-backend.h"
+#include "trace.h"
 
 #define ATAPI_SECTOR_BITS (2 + BDRV_SECTOR_BITS)
 #define ATAPI_SECTOR_SIZE (1 << ATAPI_SECTOR_BITS)
@@ -116,9 +117,7 @@ cd_read_sector_sync(IDEState *s)
 block_acct_start(blk_get_stats(s->blk), >acct,
  ATAPI_SECTOR_SIZE, BLOCK_ACCT_READ);
 
-#ifdef DEBUG_IDE_ATAPI
-printf("cd_read_sector_sync: lba=%d\n", s->lba);
-#endif
+trace_cd_read_sector_sync(s->lba);
 
 switch (s->cd_sector_size) {
 case 2048:
@@ -152,9 +151,7 @@ static void cd_read_sector_cb(void *opaque, int ret)
 {
 IDEState *s = opaque;
 
-#ifdef DEBUG_IDE_ATAPI
-printf("cd_read_sector_cb: lba=%d ret=%d\n", s->lba, ret);
-#endif
+trace_cd_read_sector_cb(s->lba, ret);
 
 if (ret < 0) {
 block_acct_failed(blk_get_stats(s->blk), >acct);
@@ -188,9 +185,7 @@ static int cd_read_sector(IDEState *s)
 s->iov.iov_len = ATAPI_SECTOR_SIZE;
 qemu_iovec_init_external(>qiov, >iov, 1);
 
-#ifdef DEBUG_IDE_ATAPI
-printf("cd_read_sector: lba=%d\n", s->lba);
-#endif
+trace_cd_read_sector(s->lba);
 
 block_acct_start(blk_get_stats(s->blk), >acct,
  ATAPI_SECTOR_SIZE, BLOCK_ACCT_READ);
@@ -213,9 +208,7 @@ void ide_atapi_cmd_ok(IDEState *s)
 
 void ide_atapi_cmd_error(IDEState *s, int sense_key, int asc)
 {
-#ifdef DEBUG_IDE_ATAPI
-printf("atapi_cmd_error: sense=0x%x asc=0x%x\n", sense_key, asc);
-#endif
+trace_ide_atapi_cmd_error(s, sense_key, asc);
 s->error = sense_key << 4;
 s->status = READY_STAT | ERR_STAT;
 s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO | ATAPI_INT_REASON_CD;
@@ -252,19 +245,14 @@ static uint16_t atapi_byte_count_limit(IDEState *s)
 void ide_atapi_cmd_reply_end(IDEState *s)
 {
 int byte_count_limit, size, ret;
-#ifdef DEBUG_IDE_ATAPI
-printf("reply: tx_size=%d elem_tx_size=%d index=%d\n",
-   s->packet_transfer_size,
-   s->elementary_transfer_size,
-   s->io_buffer_index);
-#endif
+trace_ide_atapi_cmd_reply_end(s, s->packet_transfer_size,
+  s->elementary_transfer_size,
+  s->io_buffer_index);
 if (s->packet_transfer_size <= 0) {
 /* end of transfer */
 ide_atapi_cmd_ok(s);
 ide_set_irq(s->bus);
-#ifdef DEBUG_IDE_ATAPI
-printf("end of transfer, status=0x%x\n", s->status);
-#endif
+trace_ide_atapi_cmd_reply_end_eot(s, s->status);
 } else {
 /* see if a new sector must be read */
 if (s->lba != -1 && s->io_buffer_index >= s->cd_sector_size) {
@@ -300,9 +288,7 @@ void ide_atapi_cmd_reply_end(IDEState *s)
 /* a new transfer is needed */
 s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO;
 byte_count_limit = atapi_byte_count_limit(s);
-#ifdef DEBUG_IDE_ATAPI
-printf("byte_count_limit=%d\n", byte_count_limit);
-#endif
+trace_ide_atapi_cmd_reply_end_bcl(s, byte_count_limit);
 size = s->packet_transfer_size;
 if (size > byte_count_limit) {
 /* byte count limit must be even if this case */
@@ -324,9 +310,7 @@ void ide_atapi_cmd_reply_end(IDEState *s)
 ide_transfer_start(s, s->io_buffer + s->io_buffer_index - size,
size, ide_atapi_cmd_reply_end);
 ide_set_irq(s->bus);
-#ifdef DEBUG_IDE_ATAPI
-printf("status=0x%x\n", s->status);
-#endif
+trace_ide_atapi_cmd_reply_end_new(s, s->status);
 }
 }
 }
@@ -368,9 +352,7 @@ static void ide_atapi_cmd_read_pio(IDEState *s, int lba, 
int nb_sectors,
 
 static void ide_atapi_cmd_check_status(IDEState *s)
 {
-#ifdef DEBUG_IDE_ATAPI
-printf("atapi_cmd_check_status\n");
-#endif
+trace_ide_atapi_cmd_check_status(s);
 s->error = MC_ERR | (UNIT_ATTENTION << 4);
 s->status = ERR_STAT;
 s->nsector = 0;
@@ -477,10 +459,8 @@ static void ide_atapi_cmd_read_dma(IDEState *s, int lba, 
int nb_sectors,
 static void ide_atapi_cmd_read(IDEState *s, int lba, int nb_sectors,
int sector_size)
 {
-#ifdef DEBUG_IDE_ATAPI
-printf("read %s: LBA=%d nb_sectors=%d\n", s->atapi_dma ? "dma" : "pio",
-lba, nb_sectors);
-#endif
+trace_ide_atapi_cmd_read(s, s->atapi_dma ? "dma" : "pio",
+ lba, nb_sectors);
 if 

[Qemu-devel] [PATCH v3 2/9] IDE: Add register hints to tracing

[John Snow]

Name the registers for tracing purposes.

Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
---
 hw/ide/core.c   | 88 +
 hw/ide/trace-events |  8 ++---
 2 files changed, 72 insertions(+), 24 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 31fd593..cb250e6 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1185,13 +1185,37 @@ static void ide_clear_hob(IDEBus *bus)
 bus->ifs[1].select &= ~(1 << 7);
 }
 
+/* IOport [W]rite [R]egisters */
+enum ATA_IOPORT_WR {
+ATA_IOPORT_WR_DATA = 0,
+ATA_IOPORT_WR_FEATURES = 1,
+ATA_IOPORT_WR_SECTOR_COUNT = 2,
+ATA_IOPORT_WR_SECTOR_NUMBER = 3,
+ATA_IOPORT_WR_CYLINDER_LOW = 4,
+ATA_IOPORT_WR_CYLINDER_HIGH = 5,
+ATA_IOPORT_WR_DEVICE_HEAD = 6,
+ATA_IOPORT_WR_COMMAND = 7,
+ATA_IOPORT_WR_NUM_REGISTERS,
+};
+
+const char *ATA_IOPORT_WR_lookup[ATA_IOPORT_WR_NUM_REGISTERS] = {
+[ATA_IOPORT_WR_DATA] = "Data",
+[ATA_IOPORT_WR_FEATURES] = "Features",
+[ATA_IOPORT_WR_SECTOR_COUNT] = "Sector Count",
+[ATA_IOPORT_WR_SECTOR_NUMBER] = "Sector Number",
+[ATA_IOPORT_WR_CYLINDER_LOW] = "Cylinder Low",
+[ATA_IOPORT_WR_CYLINDER_HIGH] = "Cylinder High",
+[ATA_IOPORT_WR_DEVICE_HEAD] = "Device/Head",
+[ATA_IOPORT_WR_COMMAND] = "Command"
+};
+
 void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
 {
 IDEBus *bus = opaque;
 IDEState *s = idebus_active_if(bus);
 int reg_num = addr & 7;
 
-trace_ide_ioport_write(addr, val, bus, s);
+trace_ide_ioport_write(addr, ATA_IOPORT_WR_lookup[reg_num], val, bus, s);
 
 /* ignore writes to command block while busy with previous command */
 if (reg_num != 7 && (s->status & (BUSY_STAT|DRQ_STAT))) {
@@ -1201,43 +1225,43 @@ void ide_ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 switch (reg_num) {
 case 0:
 break;
-case 1:
-   ide_clear_hob(bus);
+case ATA_IOPORT_WR_FEATURES:
+ide_clear_hob(bus);
 /* NOTE: data is written to the two drives */
-   bus->ifs[0].hob_feature = bus->ifs[0].feature;
-   bus->ifs[1].hob_feature = bus->ifs[1].feature;
+bus->ifs[0].hob_feature = bus->ifs[0].feature;
+bus->ifs[1].hob_feature = bus->ifs[1].feature;
 bus->ifs[0].feature = val;
 bus->ifs[1].feature = val;
 break;
-case 2:
+case ATA_IOPORT_WR_SECTOR_COUNT:
ide_clear_hob(bus);
bus->ifs[0].hob_nsector = bus->ifs[0].nsector;
bus->ifs[1].hob_nsector = bus->ifs[1].nsector;
 bus->ifs[0].nsector = val;
 bus->ifs[1].nsector = val;
 break;
-case 3:
+case ATA_IOPORT_WR_SECTOR_NUMBER:
ide_clear_hob(bus);
bus->ifs[0].hob_sector = bus->ifs[0].sector;
bus->ifs[1].hob_sector = bus->ifs[1].sector;
 bus->ifs[0].sector = val;
 bus->ifs[1].sector = val;
 break;
-case 4:
+case ATA_IOPORT_WR_CYLINDER_LOW:
ide_clear_hob(bus);
bus->ifs[0].hob_lcyl = bus->ifs[0].lcyl;
bus->ifs[1].hob_lcyl = bus->ifs[1].lcyl;
 bus->ifs[0].lcyl = val;
 bus->ifs[1].lcyl = val;
 break;
-case 5:
+case ATA_IOPORT_WR_CYLINDER_HIGH:
ide_clear_hob(bus);
bus->ifs[0].hob_hcyl = bus->ifs[0].hcyl;
bus->ifs[1].hob_hcyl = bus->ifs[1].hcyl;
 bus->ifs[0].hcyl = val;
 bus->ifs[1].hcyl = val;
 break;
-case 6:
+case ATA_IOPORT_WR_DEVICE_HEAD:
/* FIXME: HOB readback uses bit 7 */
 bus->ifs[0].select = (val & ~0x10) | 0xa0;
 bus->ifs[1].select = (val | 0x10) | 0xa0;
@@ -1245,7 +1269,7 @@ void ide_ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 bus->unit = (val >> 4) & 1;
 break;
 default:
-case 7:
+case ATA_IOPORT_WR_COMMAND:
 /* command */
 ide_exec_cmd(bus, val);
 break;
@@ -2052,6 +2076,30 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
 }
 }
 
+/* IOport [R]ead [R]egisters */
+enum ATA_IOPORT_RR {
+ATA_IOPORT_RR_DATA = 0,
+ATA_IOPORT_RR_ERROR = 1,
+ATA_IOPORT_RR_SECTOR_COUNT = 2,
+ATA_IOPORT_RR_SECTOR_NUMBER = 3,
+ATA_IOPORT_RR_CYLINDER_LOW = 4,
+ATA_IOPORT_RR_CYLINDER_HIGH = 5,
+ATA_IOPORT_RR_DEVICE_HEAD = 6,
+ATA_IOPORT_RR_STATUS = 7,
+ATA_IOPORT_RR_NUM_REGISTERS,
+};
+
+const char *ATA_IOPORT_RR_lookup[ATA_IOPORT_RR_NUM_REGISTERS] = {
+[ATA_IOPORT_RR_DATA] = "Data",
+[ATA_IOPORT_RR_ERROR] = "Error",
+[ATA_IOPORT_RR_SECTOR_COUNT] = "Sector Count",
+[ATA_IOPORT_RR_SECTOR_NUMBER] = "Sector Number",
+[ATA_IOPORT_RR_CYLINDER_LOW] = "Cylinder Low",
+[ATA_IOPORT_RR_CYLINDER_HIGH] = "Cylinder High",
+[ATA_IOPORT_RR_DEVICE_HEAD] = "Device/Head",
+[ATA_IOPORT_RR_STATUS] = "Status"
+};
+
 uint32_t ide_ioport_read(void *opaque, uint32_t addr)
 {
 IDEBus *bus = opaque;
@@ -2064,10 +2112,10 @@ uint32_t ide_ioport_read(void *opaque, uint32_t addr)

[Qemu-devel] [PATCH v3 3/9] IDE: add tracing for data ports

[John Snow]

To be used sparingly, but still interesting in the case of small
firmwares designed to reproduce bugs in QEMU IDE.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: John Snow 
---
 hw/ide/core.c   | 12 +++-
 hw/ide/trace-events |  5 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index cb250e6..82a19b1 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2259,6 +2259,8 @@ void ide_data_writew(void *opaque, uint32_t addr, 
uint32_t val)
 IDEState *s = idebus_active_if(bus);
 uint8_t *p;
 
+trace_ide_data_writew(addr, val, bus, s);
+
 /* PIO data access allowed only when DRQ bit is set. The result of a write
  * during PIO out is indeterminate, just ignore it. */
 if (!(s->status & DRQ_STAT) || ide_is_pio_out(s)) {
@@ -2304,6 +2306,8 @@ uint32_t ide_data_readw(void *opaque, uint32_t addr)
 s->status &= ~DRQ_STAT;
 s->end_transfer_func(s);
 }
+
+trace_ide_data_readw(addr, ret, bus, s);
 return ret;
 }
 
@@ -2313,6 +2317,8 @@ void ide_data_writel(void *opaque, uint32_t addr, 
uint32_t val)
 IDEState *s = idebus_active_if(bus);
 uint8_t *p;
 
+trace_ide_data_writel(addr, val, bus, s);
+
 /* PIO data access allowed only when DRQ bit is set. The result of a write
  * during PIO out is indeterminate, just ignore it. */
 if (!(s->status & DRQ_STAT) || ide_is_pio_out(s)) {
@@ -2343,7 +2349,8 @@ uint32_t ide_data_readl(void *opaque, uint32_t addr)
 /* PIO data access allowed only when DRQ bit is set. The result of a read
  * during PIO in is indeterminate, return 0 and don't move forward. */
 if (!(s->status & DRQ_STAT) || !ide_is_pio_out(s)) {
-return 0;
+ret = 0;
+goto out;
 }
 
 p = s->data_ptr;
@@ -2358,6 +2365,9 @@ uint32_t ide_data_readl(void *opaque, uint32_t addr)
 s->status &= ~DRQ_STAT;
 s->end_transfer_func(s);
 }
+
+out:
+trace_ide_data_readl(addr, ret, bus, s);
 return ret;
 }
 
diff --git a/hw/ide/trace-events b/hw/ide/trace-events
index bff8f39..17bc6f1 100644
--- a/hw/ide/trace-events
+++ b/hw/ide/trace-events
@@ -6,6 +6,11 @@ ide_ioport_read(uint32_t addr, const char *reg, uint32_t val, 
void *bus, void *s
 ide_ioport_write(uint32_t addr, const char *reg, uint32_t val, void *bus, void 
*s) "IDE PIO wr @ 0x%"PRIx32" (%s); val 0x%02"PRIx32"; bus %p IDEState %p"
 ide_status_read(uint32_t addr, uint32_t val, void *bus, void *s)   
"IDE PIO rd @ 0x%"PRIx32" (Alt Status); val 0x%02"PRIx32"; bus %p; IDEState 
%p"
 ide_cmd_write(uint32_t addr, uint32_t val, void *bus)  
"IDE PIO wr @ 0x%"PRIx32" (Device Control); val 0x%02"PRIx32"; bus %p"
+# Warning: verbose
+ide_data_readw(uint32_t addr, uint32_t val, void *bus, void *s)
"IDE PIO rd @ 0x%"PRIx32" (Data: Word); val 0x%04"PRIx32"; bus %p; IDEState 
%p"
+ide_data_writew(uint32_t addr, uint32_t val, void *bus, void *s)   
"IDE PIO wr @ 0x%"PRIx32" (Data: Word); val 0x%04"PRIx32"; bus %p; IDEState 
%p"
+ide_data_readl(uint32_t addr, uint32_t val, void *bus, void *s)
"IDE PIO rd @ 0x%"PRIx32" (Data: Long); val 0x%08"PRIx32"; bus %p; IDEState 
%p"
+ide_data_writel(uint32_t addr, uint32_t val, void *bus, void *s)   
"IDE PIO wr @ 0x%"PRIx32" (Data: Long); val 0x%08"PRIx32"; bus %p; IDEState 
%p"
 # misc
 ide_exec_cmd(void *bus, void *state, uint32_t cmd) "IDE exec cmd: bus %p; 
state %p; cmd 0x%02x"
 ide_cancel_dma_sync_buffered(void *fn, void *req) "invoking cb %p of buffered 
request %p with -ECANCELED"
-- 
2.9.5

[Qemu-devel] [PATCH v3 1/9] IDE: replace DEBUG_IDE with tracing system

[John Snow]

Remove the DEBUG_IDE preprocessor definition with something more
appropriately flexible, using the trace-events subsystem.

This will be less prone to bitrot and will more effectively allow
us to target just the functions we care about.

Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
---
 Makefile.objs |  1 +
 hw/ide/cmd646.c   | 10 +++-
 hw/ide/core.c | 65 +++
 hw/ide/pci.c  | 17 -
 hw/ide/piix.c | 11 
 hw/ide/trace-events   | 35 +
 hw/ide/via.c  | 10 +++-
 include/hw/ide/internal.h |  1 -
 8 files changed, 80 insertions(+), 70 deletions(-)
 create mode 100644 hw/ide/trace-events

diff --git a/Makefile.objs b/Makefile.objs
index 24a4ea0..967c092 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -153,6 +153,7 @@ trace-events-subdirs += hw/acpi
 trace-events-subdirs += hw/arm
 trace-events-subdirs += hw/alpha
 trace-events-subdirs += hw/xen
+trace-events-subdirs += hw/ide
 trace-events-subdirs += ui
 trace-events-subdirs += audio
 trace-events-subdirs += net
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 9ebb8d4..86b2a8f 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -32,6 +32,7 @@
 #include "sysemu/dma.h"
 
 #include "hw/ide/pci.h"
+#include "trace.h"
 
 /* CMD646 specific */
 #define CFR0x50
@@ -195,9 +196,8 @@ static uint64_t bmdma_read(void *opaque, hwaddr addr,
 val = 0xff;
 break;
 }
-#ifdef DEBUG_IDE
-printf("bmdma: readb " TARGET_FMT_plx " : 0x%02x\n", addr, val);
-#endif
+
+trace_bmdma_read_cmd646(addr, val);
 return val;
 }
 
@@ -211,9 +211,7 @@ static void bmdma_write(void *opaque, hwaddr addr,
 return;
 }
 
-#ifdef DEBUG_IDE
-printf("bmdma: writeb " TARGET_FMT_plx " : 0x%" PRIx64 "\n", addr, val);
-#endif
+trace_bmdma_write_cmd646(addr, val);
 switch(addr & 3) {
 case 0:
 bmdma_cmd_writeb(bm, val);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index bea3953..31fd593 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -36,6 +36,7 @@
 #include "qemu/cutils.h"
 
 #include "hw/ide/internal.h"
+#include "trace.h"
 
 /* These values were based on a Seagate ST3500418AS but have been modified
to make more sense in QEMU */
@@ -656,10 +657,7 @@ void ide_cancel_dma_sync(IDEState *s)
  * write requests) pending and we can avoid to drain. */
 QLIST_FOREACH(req, >buffered_requests, list) {
 if (!req->orphaned) {
-#ifdef DEBUG_IDE
-printf("%s: invoking cb %p of buffered request %p with"
-   " -ECANCELED\n", __func__, req->original_cb, req);
-#endif
+trace_ide_cancel_dma_sync_buffered(req->original_cb, req);
 req->original_cb(req->original_opaque, -ECANCELED);
 }
 req->orphaned = true;
@@ -678,9 +676,7 @@ void ide_cancel_dma_sync(IDEState *s)
  * aio operation with preadv/pwritev.
  */
 if (s->bus->dma->aiocb) {
-#ifdef DEBUG_IDE
-printf("%s: draining all remaining requests", __func__);
-#endif
+trace_ide_cancel_dma_sync_remaining();
 blk_drain(s->blk);
 assert(s->bus->dma->aiocb == NULL);
 }
@@ -741,9 +737,7 @@ static void ide_sector_read(IDEState *s)
 n = s->req_nb_sectors;
 }
 
-#if defined(DEBUG_IDE)
-printf("sector=%" PRId64 "\n", sector_num);
-#endif
+trace_ide_sector_read(sector_num, n);
 
 if (!ide_sect_range_ok(s, sector_num, n)) {
 ide_rw_error(s);
@@ -1005,14 +999,14 @@ static void ide_sector_write(IDEState *s)
 
 s->status = READY_STAT | SEEK_STAT | BUSY_STAT;
 sector_num = ide_get_sector(s);
-#if defined(DEBUG_IDE)
-printf("sector=%" PRId64 "\n", sector_num);
-#endif
+
 n = s->nsector;
 if (n > s->req_nb_sectors) {
 n = s->req_nb_sectors;
 }
 
+trace_ide_sector_write(sector_num, n);
+
 if (!ide_sect_range_ok(s, sector_num, n)) {
 ide_rw_error(s);
 block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_WRITE);
@@ -1194,18 +1188,17 @@ static void ide_clear_hob(IDEBus *bus)
 void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
 {
 IDEBus *bus = opaque;
+IDEState *s = idebus_active_if(bus);
+int reg_num = addr & 7;
 
-#ifdef DEBUG_IDE
-printf("IDE: write addr=0x%x val=0x%02x\n", addr, val);
-#endif
-
-addr &= 7;
+trace_ide_ioport_write(addr, val, bus, s);
 
 /* ignore writes to command block while busy with previous command */
-if (addr != 7 && (idebus_active_if(bus)->status & (BUSY_STAT|DRQ_STAT)))
+if (reg_num != 7 && (s->status & (BUSY_STAT|DRQ_STAT))) {
 return;
+}
 
-switch(addr) {
+switch (reg_num) {
 case 0:
 break;
 case 1:
@@ -1261,9 +1254,7 @@ void ide_ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 
 static void ide_reset(IDEState *s)
 {
-#ifdef DEBUG_IDE
-printf("ide: reset\n");

[Qemu-devel] [PATCH v3 5/9] IDE: replace DEBUG_AIO with trace events

[John Snow]

Signed-off-by: John Snow 
---
 hw/ide/atapi.c|  5 +
 hw/ide/core.c | 24 +---
 hw/ide/trace-events   |  3 +++
 include/hw/ide/internal.h |  6 --
 4 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 9b84a1b..c0509c8 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -416,10 +416,7 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int 
ret)
 s->io_buffer_size = n * 2048;
 data_offset = 0;
 }
-#ifdef DEBUG_AIO
-printf("aio_read_cd: lba=%u n=%d\n", s->lba, n);
-#endif
-
+trace_ide_atapi_cmd_read_dma_cb_aio(s, s->lba, n);
 s->bus->dma->iov.iov_base = (void *)(s->io_buffer + data_offset);
 s->bus->dma->iov.iov_len = n * ATAPI_SECTOR_SIZE;
 qemu_iovec_init_external(>bus->dma->qiov, >bus->dma->iov, 1);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 82a19b1..2cc2a08 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -58,6 +58,21 @@ static const int smart_attributes[][12] = {
 { 190,  0x03, 0x00, 0x45, 0x45, 0x1f, 0x00, 0x1f, 0x1f, 0x00, 0x00, 0x32},
 };
 
+const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT] = {
+[IDE_DMA_READ] = "DMA READ",
+[IDE_DMA_WRITE] = "DMA WRITE",
+[IDE_DMA_TRIM] = "DMA TRIM",
+[IDE_DMA_ATAPI] = "DMA ATAPI"
+};
+
+static const char *IDE_DMA_CMD_str(enum ide_dma_cmd enval)
+{
+if (enval >= 0 && enval < IDE_DMA__COUNT) {
+return IDE_DMA_CMD_lookup[enval];
+}
+return "DMA UNKNOWN CMD";
+}
+
 static void ide_dummy_transfer_stop(IDEState *s);
 
 static void padstr(char *str, const char *src, int len)
@@ -860,10 +875,7 @@ static void ide_dma_cb(void *opaque, int ret)
 goto eot;
 }
 
-#ifdef DEBUG_AIO
-printf("ide_dma_cb: sector_num=%" PRId64 " n=%d, cmd_cmd=%d\n",
-   sector_num, n, s->dma_cmd);
-#endif
+trace_ide_dma_cb(s, sector_num, n, IDE_DMA_CMD_str(s->dma_cmd));
 
 if ((s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) &&
 !ide_sect_range_ok(s, sector_num, n)) {
@@ -2391,9 +2403,7 @@ void ide_bus_reset(IDEBus *bus)
 
 /* pending async DMA */
 if (bus->dma->aiocb) {
-#ifdef DEBUG_AIO
-printf("aio_cancel\n");
-#endif
+trace_ide_bus_reset_aio();
 blk_aio_cancel(bus->dma->aiocb);
 bus->dma->aiocb = NULL;
 }
diff --git a/hw/ide/trace-events b/hw/ide/trace-events
index 8c79a6c..cc8949c 100644
--- a/hw/ide/trace-events
+++ b/hw/ide/trace-events
@@ -18,6 +18,8 @@ ide_cancel_dma_sync_remaining(void) "draining all remaining 
requests"
 ide_sector_read(int64_t sector_num, int nsectors) "sector=%"PRId64" 
nsectors=%d"
 ide_sector_write(int64_t sector_num, int nsectors) "sector=%"PRId64" 
nsectors=%d"
 ide_reset(void *s) "IDEstate %p"
+ide_bus_reset_aio(void) "aio_cancel"
+ide_dma_cb(void *s, int64_t sector_num, int n, const char *dma) "IDEState %p; 
sector_num=%"PRId64" n=%d cmd=%s"
 
 # BMDMA HBAs:
 
@@ -51,5 +53,6 @@ ide_atapi_cmd_reply_end_new(void *s, int status) "IDEState: 
%p; new transfer sta
 ide_atapi_cmd_check_status(void *s) "IDEState: %p"
 ide_atapi_cmd_read(void *s, const char *method, int lba, int nb_sectors) 
"IDEState: %p; read %s: LBA=%d nb_sectors=%d"
 ide_atapi_cmd(void *s, uint8_t cmd) "IDEState: %p; cmd: 0x%02x"
+ide_atapi_cmd_read_dma_cb_aio(void *s, int lba, int n) "IDEState: %p; aio 
read: lba=%d n=%d"
 # Warning: Verbose
 ide_atapi_cmd_packet(void *s, uint16_t limit, const char *packet) "IDEState: 
%p; limit=0x%x packet: %s"
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 74efe8a..db9fde0 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -14,7 +14,6 @@
 #include "block/scsi.h"
 
 /* debug IDE devices */
-//#define DEBUG_AIO
 #define USE_DMA_CDROM
 
 typedef struct IDEBus IDEBus;
@@ -333,12 +332,15 @@ struct unreported_events {
 };
 
 enum ide_dma_cmd {
-IDE_DMA_READ,
+IDE_DMA_READ = 0,
 IDE_DMA_WRITE,
 IDE_DMA_TRIM,
 IDE_DMA_ATAPI,
+IDE_DMA__COUNT
 };
 
+extern const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT];
+
 #define ide_cmd_is_read(s) \
((s)->dma_cmd == IDE_DMA_READ)
 
-- 
2.9.5

[Qemu-devel] [PATCH v3 0/9] IDE: replace printfs with tracing

[John Snow]

Wherever possible, replace all printfs with proper tracing.
In most places I've tried to do a straight replacement, but
forthcoming patches may calibrate the tracing to be a little nicer.

For now, it's nice to just remove the all-or-nothing tracing.

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/9:[] [--] 'IDE: replace DEBUG_IDE with tracing system'
002/9:[] [--] 'IDE: Add register hints to tracing'
003/9:[] [--] 'IDE: add tracing for data ports'
004/9:[0002] [FC] 'ATAPI: Replace DEBUG_IDE_ATAPI with tracing events'
005/9:[0011] [FC] 'IDE: replace DEBUG_AIO with trace events'
006/9:[] [--] 'AHCI: Replace DPRINTF with trace-events'
007/9:[0004] [FC] 'AHCI: Rework IRQ constants'
008/9:[0052] [FC] 'AHCI: pretty-print FIS to buffer instead of stderr'
009/9:[] [--] 'AHCI: remove DPRINTF macro'

===
v3:
===

04: Change preprocessor gate mechanism for building cmd
   packet buffer dumps (Stefan)
05: Add getter for IDE_DMA_CMD enumeration (Philippe)
07: Changed AHCI enumeration to match changes in 05,
(But leave assert in place to guard lookup table instead
 of adding getter.)
08: Replace FIS buffer printer with something simpler (Stefan)
Change preprocessor gate mechanism for building AHCI buffer dumps (Stefan)
enconstify char parameters in trace-events file (Stefan)

===
v2:
===

01: Rehabilitate commit message.
Fix switch () statement spacing. (Eric)
Adjust spacing in ide-tracing file to maintain columns.
Change filename orderings in trace-events. (Eric)
Fixed newline issue in trace-events. (Eric)

02: Context / Added R-B.

03: Added trace-events items to appropriate patch (removed from 04) (Eric)
Added a verbose warning for data tracers (Eric)

04: Shifted items to 03 (Eric)
Fixed newline issue (Eric)

05: Changed __END to __COUNT (Philippe)
Removed __BEGIN enumerator (Philippe)

06: Added more information to unknown write (Philippe)

08: Context (Entropy)

John Snow (9):
  IDE: replace DEBUG_IDE with tracing system
  IDE: Add register hints to tracing
  IDE: add tracing for data ports
  ATAPI: Replace DEBUG_IDE_ATAPI with tracing events
  IDE: replace DEBUG_AIO with trace events
  AHCI: Replace DPRINTF with trace-events
  AHCI: Rework IRQ constants
  AHCI: pretty-print FIS to buffer instead of stderr
  AHCI: remove DPRINTF macro

 Makefile.objs |   1 +
 hw/ide/ahci.c | 243 +++---
 hw/ide/ahci_internal.h|  44 +++--
 hw/ide/atapi.c|  69 +
 hw/ide/cmd646.c   |  10 +-
 hw/ide/core.c | 185 ++-
 hw/ide/pci.c  |  17 +---
 hw/ide/piix.c |  11 +--
 hw/ide/trace-events   | 111 +
 hw/ide/via.c  |  10 +-
 include/hw/ide/internal.h |   8 +-
 11 files changed, 437 insertions(+), 272 deletions(-)
 create mode 100644 hw/ide/trace-events

-- 
2.9.5

Re: [Qemu-devel] [PATCH 02/14] qlit: move qlit from check-qjson to qobject/

[Eduardo Habkost]

On Thu, Aug 24, 2017 at 12:33:38PM +0200, Marc-André Lureau wrote:
> Fix code style issues while at it, to please check-patch.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  include/qapi/qmp/qlit.h | 49 +
>  qobject/qlit.c  | 89 +
>  tests/check-qjson.c | 96 
> +
>  qobject/Makefile.objs   |  2 +-
>  4 files changed, 140 insertions(+), 96 deletions(-)
>  create mode 100644 include/qapi/qmp/qlit.h
>  create mode 100644 qobject/qlit.c
> 
> diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
> new file mode 100644
> index 00..4e2e760ef1
> --- /dev/null
> +++ b/include/qapi/qmp/qlit.h
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright IBM, Corp. 2009
> + * Copyright (c) 2013, 2015, 2017 Red Hat Inc.
> + *
> + * Authors:
> + *  Anthony Liguori   
> + *  Markus Armbruster 
> + *  Marc-André Lureau 
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or 
> later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +#ifndef QLIT_H_
> +#define QLIT_H_
> +
> +#include "qapi-types.h"
> +#include "qobject.h"
> +
> +typedef struct LiteralQDictEntry LiteralQDictEntry;
> +typedef struct LiteralQObject LiteralQObject;
> +
> +struct LiteralQObject {
> +int type;
> +union {
> +int64_t qnum;
> +const char *qstr;
> +LiteralQDictEntry *qdict;
> +LiteralQObject *qlist;
> +} value;
> +};
> +
> +struct LiteralQDictEntry {
> +const char *key;
> +LiteralQObject value;
> +};
> +
> +#define QLIT_QNUM(val) \
> +(LiteralQObject){.type = QTYPE_QNUM, .value.qnum = (val)}
> +#define QLIT_QSTR(val) \
> +(LiteralQObject){.type = QTYPE_QSTRING, .value.qstr = (val)}
> +#define QLIT_QDICT(val) \
> +(LiteralQObject){.type = QTYPE_QDICT, .value.qdict = (val)}
> +#define QLIT_QLIST(val) \
> +(LiteralQObject){.type = QTYPE_QLIST, .value.qlist = (val)}

I'm still trying to understand why this exists.  Doesn't this
provide exactly the same functionality as QObject?

> [...]

-- 
Eduardo

Re: [Qemu-devel] [Qemu devel v7 PATCH 4/5] msf2: Add Smartfusion2 SoC

[Alistair Francis]

On Thu, Aug 31, 2017 at 4:40 PM, Philippe Mathieu-Daudé  wrote:
> Hi Alistair,
>
>
> On 08/31/2017 08:02 PM, Alistair Francis wrote:
>>
>> On Wed, Aug 30, 2017 at 7:47 AM, Philippe Mathieu-Daudé 
>> wrote:
>>>
>>> On 08/30/2017 09:26 AM, Peter Maydell wrote:


 On 30 August 2017 at 03:45, Philippe Mathieu-Daudé 
 wrote:
>
>
> I think they might be issues if you start QEMU without -serial and then
> use
> a firmware polling for an uart, the device won't be mapped and the
> memory
> accesses are mostly ignored.
>
> I'd rather use:
>
>   for (i = 0; i < MSF2_NUM_UARTS && i < MAX_SERIAL_PORTS; i++) {
>   static const char *serial[] = {"serial0", "serial1"};
>
>   if (!serial_hds[i]) {
>   serial_hds[i] = qemu_chr_new(serial[i], "null");
>
>   }
>
>> +serial_mm_init(get_system_memory(), uart_addr[i], 2,
>> +   qdev_get_gpio_in(armv7m, uart_irq[i]),
>> +   115200, serial_hds[i],
>> DEVICE_NATIVE_ENDIAN);
>> +}
>> +}



 It would be better to fix serial_mm_init() to handle having
 a NULL chardev pointer, because we already have a lot of
 SoC code that just passes it serial_hds[] regardless.
>>>
>>>
>>>
>>> clever :)
>>>
 I'd leave this code as it is and we can fix serial_mm_init
 separately (somebody pointed out this issue for a xilinx
 board recently).
>>
>>
>> Ah, I'll look into this then.
>
>
> I already sent a series to take care of this:
>
> http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg06325.html

That was quick.

Can you CC me on the next version?

Thanks,
Alistair

>
> I'll respin a v2 shortly.

[Qemu-devel] [PATCH v2 4/5] xlnx-zcu102: Add a machine level virtualization property

[Alistair Francis]

Add a machine level virtualization property. This defaults to false and can be
set to true using this machine command line argument:
-machine xlnx-zcu102,virtualization=on

This follows what the ARM virt machine does.

This property only applies to the ZCU102 machine. The EP108 machine does
not have this property.

Signed-off-by: Alistair Francis 
---

 hw/arm/xlnx-zcu102.c | 30 +-
 hw/arm/xlnx-zynqmp.c |  3 ++-
 include/hw/arm/xlnx-zynqmp.h |  2 ++
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 080507831a..40c1f5bbf6 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -32,6 +32,7 @@ typedef struct XlnxZCU102 {
 MemoryRegion ddr_ram;
 
 bool secure;
+bool virt;
 } XlnxZCU102;
 
 #define TYPE_ZCU102_MACHINE   MACHINE_TYPE_NAME("xlnx-zcu102")
@@ -58,6 +59,20 @@ static void zcu102_set_secure(Object *obj, bool value, Error 
**errp)
 s->secure = value;
 }
 
+static bool zcu102_get_virt(Object *obj, Error **errp)
+{
+XlnxZCU102 *s = ZCU102_MACHINE(obj);
+
+return s->virt;
+}
+
+static void zcu102_set_virt(Object *obj, bool value, Error **errp)
+{
+XlnxZCU102 *s = ZCU102_MACHINE(obj);
+
+s->virt = value;
+}
+
 static void xlnx_zynqmp_init(XlnxZCU102 *s, MachineState *machine)
 {
 int i;
@@ -87,6 +102,8 @@ static void xlnx_zynqmp_init(XlnxZCU102 *s, MachineState 
*machine)
  "ddr-ram", _abort);
 object_property_set_bool(OBJECT(>soc), s->secure, "secure",
  _fatal);
+object_property_set_bool(OBJECT(>soc), s->virt, "virtualization",
+ _fatal);
 
 object_property_set_bool(OBJECT(>soc), true, "realized", _fatal);
 
@@ -154,8 +171,9 @@ static void xlnx_ep108_machine_instance_init(Object *obj)
 {
 XlnxZCU102 *s = EP108_MACHINE(obj);
 
-/* EP108, we don't support setting secure */
+/* EP108, we don't support setting secure or virt */
 s->secure = false;
+s->virt = false;
 }
 
 static void xlnx_ep108_machine_class_init(ObjectClass *oc, void *data)
@@ -200,6 +218,16 @@ static void xlnx_zcu102_machine_instance_init(Object *obj)
 "Set on/off to enable/disable the ARM "
 "Security Extensions (TrustZone)",
 NULL);
+
+/* Default to virt (EL2) being disabled */
+s->virt = false;
+object_property_add_bool(obj, "virtualization", zcu102_get_virt,
+ zcu102_set_virt, NULL);
+object_property_set_description(obj, "virtualization",
+"Set on/off to enable/disable emulating a "
+"guest CPU which implements the ARM "
+"Virtualization Extensions",
+NULL);
 }
 
 static void xlnx_zcu102_machine_class_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 9eceadbdc8..37a8bf2ccd 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -260,7 +260,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 object_property_set_bool(OBJECT(>apu_cpu[i]),
  s->secure, "has_el3", NULL);
 object_property_set_bool(OBJECT(>apu_cpu[i]),
- false, "has_el2", NULL);
+ s->virt, "has_el2", NULL);
 object_property_set_int(OBJECT(>apu_cpu[i]), GIC_BASE_ADDR,
 "reset-cbar", _abort);
 object_property_set_bool(OBJECT(>apu_cpu[i]), true, "realized",
@@ -432,6 +432,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 static Property xlnx_zynqmp_props[] = {
 DEFINE_PROP_STRING("boot-cpu", XlnxZynqMPState, boot_cpu),
 DEFINE_PROP_BOOL("secure", XlnxZynqMPState, secure, false),
+DEFINE_PROP_BOOL("virtualization", XlnxZynqMPState, virt, false),
 DEFINE_PROP_BOOL("has_rpu", XlnxZynqMPState, has_rpu, false),
 DEFINE_PROP_END_OF_LIST()
 };
diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index c2931bf39c..6eff81a995 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -91,6 +91,8 @@ typedef struct XlnxZynqMPState {
 
 /* Has the ARM Security extensions?  */
 bool secure;
+/* Has the ARM Virtualization extensions?  */
+bool virt;
 /* Has the RPU subsystem?  */
 bool has_rpu;
 }  XlnxZynqMPState;
-- 
2.11.0

Re: [Qemu-devel] [Qemu devel v7 PATCH 4/5] msf2: Add Smartfusion2 SoC

[Philippe Mathieu-Daudé]

Hi Alistair,

On 08/31/2017 08:02 PM, Alistair Francis wrote:

On Wed, Aug 30, 2017 at 7:47 AM, Philippe Mathieu-Daudé  wrote:

On 08/30/2017 09:26 AM, Peter Maydell wrote:


On 30 August 2017 at 03:45, Philippe Mathieu-Daudé 
wrote:


I think they might be issues if you start QEMU without -serial and then
use
a firmware polling for an uart, the device won't be mapped and the memory
accesses are mostly ignored.

I'd rather use:

  for (i = 0; i < MSF2_NUM_UARTS && i < MAX_SERIAL_PORTS; i++) {
  static const char *serial[] = {"serial0", "serial1"};

  if (!serial_hds[i]) {
  serial_hds[i] = qemu_chr_new(serial[i], "null");

  }


+serial_mm_init(get_system_memory(), uart_addr[i], 2,
+   qdev_get_gpio_in(armv7m, uart_irq[i]),
+   115200, serial_hds[i],
DEVICE_NATIVE_ENDIAN);
+}
+}



It would be better to fix serial_mm_init() to handle having
a NULL chardev pointer, because we already have a lot of
SoC code that just passes it serial_hds[] regardless.



clever :)


I'd leave this code as it is and we can fix serial_mm_init
separately (somebody pointed out this issue for a xilinx
board recently).


Ah, I'll look into this then.


I already sent a series to take care of this:

http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg06325.html

I'll respin a v2 shortly.

[Qemu-devel] [PATCH v2 0/5] Expose the secure and virt properties to the

[Alistair Francis]

The EL2 and EL3 work is working well now and interanlly we now have
tests that expect to start in EL3 and transition through EL2 to EL1. To
make this easy to run let's expose the secure property to the machine
and also add a virt property that can enable EL2.

This series also does some machine/name tidying up and makes the first
move to deprecating the EP108 machine, which was just an early access
development board.

V2:
 - Add a virt option for setting EL2



Alistair Francis (5):
  xlnx-ep108: Rename to ZCU102
  xlnx-zcu102: Manually create the machines
  xlnx-zcu102: Add a machine level secure property
  xlnx-zcu102: Add a machine level virtualization property
  xlnx-zcu102: Mark the EP108 machine as deprecated

 hw/arm/Makefile.objs |   2 +-
 hw/arm/xlnx-ep108.c  | 137 ---
 hw/arm/xlnx-zcu102.c | 257 +++
 hw/arm/xlnx-zynqmp.c |   3 +-
 include/hw/arm/xlnx-zynqmp.h |   2 +
 5 files changed, 262 insertions(+), 139 deletions(-)
 delete mode 100644 hw/arm/xlnx-ep108.c
 create mode 100644 hw/arm/xlnx-zcu102.c

-- 
2.11.0

[Qemu-devel] [PATCH v2 5/5] xlnx-zcu102: Mark the EP108 machine as deprecated

[Alistair Francis]

The EP108 is the same as the ZCU102, mark it as deprecated as we don't
need two machines.

Signed-off-by: Alistair Francis 
---

 hw/arm/xlnx-zcu102.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 40c1f5bbf6..d3694f67db 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -180,7 +180,7 @@ static void xlnx_ep108_machine_class_init(ObjectClass *oc, 
void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
 
-mc->desc = "Xilinx ZynqMP EP108 board";
+mc->desc = "Xilinx ZynqMP EP108 board (Deprecated, please use 
xlnx-zcu102)";
 mc->init = xlnx_ep108_init;
 mc->block_default_type = IF_IDE;
 mc->units_per_default_bus = 1;
-- 
2.11.0

Re: [Qemu-devel] [PATCH v7 00/11] scripts/qemu.py fixes and cleanups

[Eduardo Habkost]

I was going to queue most of this series except patch 02/11 due
the "qemu received signal" message log level, but a few conflicts
prevent the rest of the series from applying cleanly.  Are you
planning to send v8?

On Fri, Aug 18, 2017 at 07:05:15PM +0200, Amador Pahim wrote:
> Changes v1->v2:
>  - Style fixes to make checkpatch.pl happy.
>  - Rebased.
> Changes v2->v3:
>  - Fix typo in patch 3 ("qemu.py: make 'args' public") commit message.
> Changes v3->v4:
>  - Squash the 2 first commits since they are co-dependant.
>  - Cleanup launch() and shutdown().
>  - Reorder the commits, putting the rename of self._args first.
>  - Rebased.
> Changes v4->v5:
>  - Break the cleanup commit into logical changes and include in the
>commit messages the rationale for making them.
> Changes v5->v6:
>  - Remove the commit to rename self._args.
>  - Fix is_running() return before first call to maunch().
>  - Use python logging system.
>  - Include the full command line on negative exit code error message.
>  - Use os.path.null instead of /dev/null.
>  - Improve the control over the created/deleted files. 
> Changes v6->v7:
>  - Split commits in self-contained/atomic changes.
>  - Addressed the comments from previous version, basically improving the
>logging messages and the control over created files. See individual
>commit messages for details.
> 
> Amador Pahim (11):
>   qemu.py: fix is_running() return before first launch()
>   qemu.py: avoid writing to stdout/stderr
>   qemu.py: use os.path.null instead of /dev/null
>   qemu.py: use poll() instead of 'returncode'
>   qemu.py: cleanup redundant calls in launch()
>   qemu.py: improve message on negative exit code
>   qemu.py: include debug information on launch error
>   qemu.py: make sure we only remove files we create
>   qemu.py: launch vm only if it's not running
>   qemu.py: don't launch again before shutdown()
>   qemu.py: refactor launch()
> 
>  scripts/qemu.py | 141 
> ++--
>  1 file changed, 106 insertions(+), 35 deletions(-)
> 
> -- 
> 2.13.5
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [Qemu devel v7 PATCH 4/5] msf2: Add Smartfusion2 SoC

[Alistair Francis]

On Wed, Aug 30, 2017 at 7:47 AM, Philippe Mathieu-Daudé  wrote:
> On 08/30/2017 09:26 AM, Peter Maydell wrote:
>>
>> On 30 August 2017 at 03:45, Philippe Mathieu-Daudé 
>> wrote:
>>>
>>> I think they might be issues if you start QEMU without -serial and then
>>> use
>>> a firmware polling for an uart, the device won't be mapped and the memory
>>> accesses are mostly ignored.
>>>
>>> I'd rather use:
>>>
>>>  for (i = 0; i < MSF2_NUM_UARTS && i < MAX_SERIAL_PORTS; i++) {
>>>  static const char *serial[] = {"serial0", "serial1"};
>>>
>>>  if (!serial_hds[i]) {
>>>  serial_hds[i] = qemu_chr_new(serial[i], "null");
>>>
>>>  }
>>>
 +serial_mm_init(get_system_memory(), uart_addr[i], 2,
 +   qdev_get_gpio_in(armv7m, uart_irq[i]),
 +   115200, serial_hds[i],
 DEVICE_NATIVE_ENDIAN);
 +}
 +}
>>
>>
>> It would be better to fix serial_mm_init() to handle having
>> a NULL chardev pointer, because we already have a lot of
>> SoC code that just passes it serial_hds[] regardless.
>
>
> clever :)
>
>> I'd leave this code as it is and we can fix serial_mm_init
>> separately (somebody pointed out this issue for a xilinx
>> board recently).

Ah, I'll look into this then.

Thanks,
Alistair

>
>
> Sure.

Re: [Qemu-devel] [Qemu devel v7 PATCH 2/5] msf2: Microsemi Smartfusion2 System Register block

[Alistair Francis]

On Mon, Aug 28, 2017 at 9:38 AM, Subbaraya Sundeep
 wrote:
> Added Sytem register block of Smartfusion2.
> This block has PLL registers which are accessed by guest.
>
> Signed-off-by: Subbaraya Sundeep 
> ---
>  hw/misc/Makefile.objs |   1 +
>  hw/misc/msf2-sysreg.c | 199 
> ++
>  include/hw/misc/msf2-sysreg.h |  78 +
>  3 files changed, 278 insertions(+)
>  create mode 100644 hw/misc/msf2-sysreg.c
>  create mode 100644 include/hw/misc/msf2-sysreg.h
>
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 29fb922..e8f0a02 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -59,3 +59,4 @@ obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
>  obj-$(CONFIG_AUX) += auxbus.o
>  obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
>  obj-y += mmio_interface.o
> +obj-$(CONFIG_MSF2) += msf2-sysreg.o
> diff --git a/hw/misc/msf2-sysreg.c b/hw/misc/msf2-sysreg.c
> new file mode 100644
> index 000..2aeb555
> --- /dev/null
> +++ b/hw/misc/msf2-sysreg.c
> @@ -0,0 +1,199 @@
> +/*
> + * System Register block model of Microsemi SmartFusion2.
> + *
> + * Copyright (c) 2017 Subbaraya Sundeep 
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "hw/misc/msf2-sysreg.h"
> +
> +#ifndef MSF2_SYSREG_ERR_DEBUG
> +#define MSF2_SYSREG_ERR_DEBUG  0
> +#endif
> +
> +#define DB_PRINT_L(lvl, fmt, args...) do { \
> +if (MSF2_SYSREG_ERR_DEBUG >= lvl) { \
> +qemu_log("%s: " fmt "\n", __func__, ## args); \
> +} \
> +} while (0);
> +
> +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
> +
> +static inline int msf2_divbits(uint32_t div)
> +{
> +int ret = 0;
> +
> +switch (div) {
> +case 1:
> +ret = 0;
> +break;
> +case 2:
> +ret = 1;
> +break;
> +case 4:
> +ret = 2;
> +break;
> +case 8:
> +ret = 4;
> +break;
> +case 16:
> +ret = 5;
> +break;
> +case 32:
> +ret = 6;
> +break;
> +default:
> +break;
> +}
> +
> +return ret;
> +}
> +
> +static void msf2_sysreg_reset(DeviceState *d)
> +{
> +MSF2SysregState *s = MSF2_SYSREG(d);
> +
> +DB_PRINT("RESET");
> +
> +s->regs[MSSDDR_PLL_STATUS_LOW_CR] = 0x021A2358;
> +s->regs[MSSDDR_PLL_STATUS] = 0x3;
> +s->regs[MSSDDR_FACC1_CR] = msf2_divbits(s->apb0div) << 5 |
> +   msf2_divbits(s->apb1div) << 2;
> +}
> +
> +static uint64_t msf2_sysreg_read(void *opaque, hwaddr offset,
> +unsigned size)
> +{
> +MSF2SysregState *s = opaque;
> +uint32_t ret = 0;
> +
> +offset >>= 2;
> +if (offset < ARRAY_SIZE(s->regs)) {
> +ret = s->regs[offset];
> +DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx32,
> +offset << 2, ret);
> +} else {
> +qemu_log_mask(LOG_GUEST_ERROR,
> +"%s: Bad offset 0x%08" HWADDR_PRIx "\n", __func__,
> +offset << 2);
> +}
> +
> +return ret;
> +}
> +
> +static void msf2_sysreg_write(void *opaque, hwaddr offset,
> +  uint64_t val, unsigned size)
> +{
> +MSF2SysregState *s = (MSF2SysregState *)opaque;

Drop this cast

> +uint32_t newval = val;
> +uint32_t oldval;
> +
> +DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx64,
> +offset, val);
> +
> +offset >>= 2;
> +
> +switch (offset) {
> +case MSSDDR_PLL_STATUS:
> +break;
> +
> +case ESRAM_CR:
> +oldval = s->regs[ESRAM_CR];
> +if (oldval ^ newval) {

Isn't this just:
if (newval != s->regs[ESRAM_CR])

I think putting it all in the if makes it clearer.

> +qemu_log_mask(LOG_GUEST_ERROR,
> +   TYPE_MSF2_SYSREG": eSRAM remapping not supported\n");
> +}
> +break;
> +
> +case DDR_CR:
> +oldval = s->regs[DDR_CR];
> +if (oldval ^ newval) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> +   TYPE_MSF2_SYSREG": DDR remapping not supported\n");
> +}
> +break;
> +
> +case ENVM_REMAP_BASE_CR:
> +oldval = s->regs[ENVM_REMAP_BASE_CR];
> +if (oldval ^ newval) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> +   TYPE_MSF2_SYSREG": eNVM remapping not supported\n");
> +}
> +break;
> +
> +default:
> +if (offset < ARRAY_SIZE(s->regs)) {
> +

[Qemu-devel] [Bug 1705118] Re: qemu user mode does not support catching SIGSEGV on some architectures

[Bruno Haible]

The behaviour in qemu-2.10 is the same as in qemu-2.9.

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

Title:
  qemu user mode does not support catching SIGSEGV on some architectures

Status in QEMU:
  New

Bug description:
  The documentation
   says that
  qemu in user mode supports POSIX signal handling.

  Catching SIGSEGV according to POSIX, however, does not work on
ppc, ppc64, ppc64le, s390x, sparc64.
  It does work, however, on
aarch64, alpha, arm, hppa, m68k, mips, mips64, sh4.

  How to reproduce:
  The attached program runs fine (exits with code 0) on
- real hardware Linux/PowerPC64 (in 32-bit and 64-bit mode),
- real hardware Linux/PowerPC64LE,
- qemu-system-s390x emulated Linux/s390x,
- real hardware Linux/SPARC64.
  $ gcc -O -Wall testsigsegv.c; ./a.out; echo $?
  0

  For ppc:
  $ powerpc-linux-gnu-gcc-5 -O -Wall -static testsigsegv.c -o testsigsegv-ppc
  $ ~/inst-qemu/2.9.0/bin/qemu-ppc testsigsegv-ppc
  $ echo $?
  3

  For ppc64:
  $ powerpc64-linux-gnu-gcc-5 -O -Wall -static testsigsegv.c -o 
testsigsegv-ppc64
  $ ~/inst-qemu/2.9.0/bin/qemu-ppc64 testsigsegv-ppc64
  $ echo $?
  3

  For ppc64le:
  $ powerpc64le-linux-gnu-gcc-5 -O -Wall -static testsigsegv.c -o 
testsigsegv-ppc64le
  $ ~/inst-qemu/2.9.0/bin/qemu-ppc64le testsigsegv-ppc64le
  $ echo $?
  3

  For s390x:
  $ s390x-linux-gnu-gcc-5 -O -Wall -static testsigsegv.c -o testsigsegv-s390x
  $ ~/inst-qemu/2.9.0/bin/qemu-s390x testsigsegv-s390x
  $ echo $?
  3
  $ s390x-linux-gnu-gcc-5 -O -Wall -static testsigsegv.c 
-DAVOID_LINUX_S390X_COMPAT -o testsigsegv-s390x-a
  $ ~/inst-qemu/2.9.0/bin/qemu-s390x testsigsegv-s390x-a
  $ echo $?
  0
  So, the test fails here because the Linux/s390x kernel omits the least
  significant 12 bits of the fault address in the 'si_addr' field. But
  qemu-s390x is not compatible with the Linux/s390x behaviour: it puts
  the complete fault address in the 'si_addr' field.

  For sparc64:
  $ sparc64-linux-gnu-gcc-5 -O -Wall -static testsigsegv.c -o 
testsigsegv-sparc64
  $ ~/inst-qemu/2.9.0/bin/qemu-sparc64 testsigsegv-sparc64
  Segmentation fault (core dumped)

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

[Qemu-devel] [Bug 1704658] Re: O_CLOEXEC not handled in dup3 system call in user mode

[Bruno Haible]

The behaviour in qemu-2.10 is the same as in qemu-2.9.

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

Title:
  O_CLOEXEC not handled in dup3 system call in user mode

Status in QEMU:
  New

Bug description:
  In qemu user mode, for hppa and sparc64 targets, the parameter of the
  dup3 is not passed correctly when it contains the O_CLOEXEC flag.

  When the attached program runs, the expected output is:
  errno=9=EBADF

  How to reproduce on hppa:
  - Compile the program: hppa-linux-gnu-gcc-5 -O -Wall -static testdup3.c -o 
testdup3-hppa
  - Set environment variables for running qemu-hppa.
  - ~/inst-qemu/2.9.0/bin/qemu-hppa testdup3-hppa
  errno=22=EINVAL
  testdup3.c:54: assertion 'errno == EBADF' failed

  How to reproduce on sparc64:
  - Compile the program: sparc64-linux-gnu-gcc-5 -O -Wall -static testdup3.c -o 
testdup3-sparc64
  - Set environment variables for running qemu-sparc64.
  - ~/inst-qemu/2.9.0/bin/qemu-sparc64 testdup3-sparc64
  errno=22=EINVAL
  testdup3.c:54: assertion 'errno == EBADF' failed

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

[Qemu-devel] [Bug 1701971] Re: multithreading not working right under qemu user mode for sh4

[Bruno Haible]

This works fine in qemu-2.10:
$ ~/inst-qemu/2.10.0/bin/qemu-sh4 test-tls
...
Worker 0xecdff4c0 doing value swapping
Worker 0xecdff4c0 doing value swapping
Worker 0xecdff4c0 before final verify
Worker 0xecdff4c0 after  final verify
Worker 0xecdff4c0 dying.
 OK


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

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

Title:
  multithreading not working right under qemu user mode for sh4

Status in QEMU:
  Fix Released

Bug description:
  In a multithreaded program running under qemu-sh4 (version 2.9.0),
  thread termination and/or pthread_join is not working right.

  The attached program works natively on all kinds of platforms, and
  under qemu user mode emulation for at least alpha, armelhf, aarch64,
  powerpc64le.

  How to reproduce:
  - Compile the program: sh4-linux-gnu-gcc-5 -O -Wall -lpthread -o test-tls 
test-tls.c
  - Set environment variables for running qemu-sh4.
  - ~/inst-qemu/2.9.0/bin/qemu-sh4 test-tls

  Expected behaviour: After the "Worker x dying" line, the main()
  function prints "OK", and the program terminates.

  Actual behaviour (only on sh4): After the "Worker x dying" line, it 
hangs. Attaching gdb to qemu shows 15 threads with a stack trace like
  #0  safe_syscall_base () at 
/build/qemu-2.9.0/linux-user/host/x86_64/safe-syscall.inc.S:75
  #1  0x5584f86f4c48 in safe_futex (uaddr=, op=op@entry=128, 
val=val@entry=2, timeout=, uaddr2=uaddr2@entry=0x0, 
  val3=val3@entry=-161181992) at /build/qemu-2.9.0/linux-user/syscall.c:921
  #2  0x5584f870353b in do_futex (val3=-161181992, uaddr2=4134624624, 
timeout=0, val=, op=, uaddr=)
  at /build/qemu-2.9.0/linux-user/syscall.c:7147
  #3  do_syscall (cpu_env=, num=240, arg1=, 
arg2=, arg3=, arg4=0, arg5=-160342672, 
  arg6=-161181992, arg7=0, arg8=0) at 
/build/qemu-2.9.0/linux-user/syscall.c:11692
  #4  0x5584f86f454a in cpu_loop (env=env@entry=0x5584fb3d04f8) at 
/build/qemu-2.9.0/linux-user/main.c:2676
  #5  0x5584f86f5dd5 in clone_func (arg=0x7fff4d485c20) at 
/build/qemu-2.9.0/linux-user/syscall.c:6234
  #6  0x7f08f05a46ba in start_thread (arg=0x7f08f1368700) at 
pthread_create.c:333
  #7  0x7f08f02da3dd in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:109

  and 1 thread with a stack trace like
  #0  safe_syscall_base () at 
/build/qemu-2.9.0/linux-user/host/x86_64/safe-syscall.inc.S:75
  #1  0x5584f86f4c48 in safe_futex (uaddr=, op=op@entry=0, 
val=val@entry=18875, timeout=, uaddr2=uaddr2@entry=0x0, 
  val3=val3@entry=-161180376) at /build/qemu-2.9.0/linux-user/syscall.c:921
  #2  0x5584f870353b in do_futex (val3=-161180376, uaddr2=4135101768, 
timeout=0, val=, op=, uaddr=)
  at /build/qemu-2.9.0/linux-user/syscall.c:7147
  #3  do_syscall (cpu_env=, num=240, arg1=, 
arg2=, arg3=, arg4=0, arg5=-159865528, 
  arg6=-161180376, arg7=0, arg8=0) at 
/build/qemu-2.9.0/linux-user/syscall.c:11692
  #4  0x5584f86f454a in cpu_loop (env=0x5584fb3b99a8) at 
/build/qemu-2.9.0/linux-user/main.c:2676
  #5  0x5584f86c12d3 in main (argc=, argv=0x7fff4d4878b8, 
envp=)
  at /build/qemu-2.9.0/linux-user/main.c:4860

  and 1 thread with a stack trace like
  #0  syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
  #1  0x5584f876eab5 in qemu_futex_wait (val=, f=) at /build/qemu-2.9.0/include/qemu/futex.h:26
  #2  qemu_event_wait (ev=ev@entry=0x5584faa43d84 ) at 
/build/qemu-2.9.0/util/qemu-thread-posix.c:399
  #3  0x5584f87748ce in call_rcu_thread (opaque=) at 
/build/qemu-2.9.0/util/rcu.c:249
  #4  0x7f08f05a46ba in start_thread (arg=0x7f08eff62700) at 
pthread_create.c:333
  #5  0x7f08f02da3dd in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:109

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

[Qemu-devel] [Bug 1701974] Re: pwrite does not work right under qemu-sh4

[Bruno Haible]

The behaviour in qemu-2.10 is the same as in qemu-2.9.

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

Title:
  pwrite does not work right under qemu-sh4

Status in QEMU:
  New

Bug description:
  The pwrite system call has no effect when writing to a non-zero file
  position, in a program running under qemu-sh4 (version 2.9.0).

  How to reproduce:
  - Compile the program:
sh4-linux-gnu-gcc-5 -O -Wall -static -o test-pwrite test-pwrite.c
  - Set environment variable for using qemu-sh4 (actually not needed, since the 
program is statically linked here).
  - ~/inst-qemu/2.9.0/bin/qemu-sh4 test-pwrite

  Expected output:
  buf = 01W3456789

  Actual output:
  buf = 0123456789
  test-pwrite.c:56: assertion 'strcmp ("01W3456789",buf) == 0' failed
  qemu: uncaught target signal 6 (Aborted) - core dumped

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

[Qemu-devel] [Bug 1701973] Re: pread does not work right under qemu-sh4

[Bruno Haible]

The behaviour in qemu-2.10 is the same as in qemu-2.9.

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

Title:
  pread does not work right under qemu-sh4

Status in QEMU:
  New

Bug description:
  The pread system call returns a wrong value in some case, in a program
  running under qemu-sh4 (version 2.9.0).

  How to reproduce:
  - Compile the program:
sh4-linux-gnu-gcc-5 -O -Wall -static -o test-pread test-pread.c
  - Set environment variable for using qemu-sh4 (actually not needed, since the 
program is statically linked here).
  - ~/inst-qemu/2.9.0/bin/qemu-sh4 test-pread

  Expected output:
  ret=1 errno=0

  Actual output:
  ret=0 errno=2
  test-pread.c:44: assertion 'ret == 1' failed
  qemu: uncaught target signal 6 (Aborted) - core dumped

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

[Qemu-devel] [Bug 1704638] Re: weak symbol access makes qemu in user mode hang for mips, mips64

[Bruno Haible]

The behaviour in qemu-2.10 is the same as in qemu-2.9.

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

Title:
  weak symbol access makes qemu in user mode hang for mips, mips64

Status in QEMU:
  New

Bug description:
  A program that is statically linked and invokes a weak pointer should
  crash (because the weak pointer evaluates to NULL).

  With qemu in user mode, for mips and mips64, it hangs. The process
  needs to be killed with "kill -9".

  How to reproduce for mips:
  - Compile the program: mips-linux-gnu-gcc-5 -O -Wall -static -o 
testpthsigmask-mips testpthsigmask.c -pthread
  - Set environment variables for running qemu-mips.
  - ~/inst-qemu/2.9.0/bin/qemu-mips testpthsigmask-mips

  How to reproduce for mips64:
  - Compile the program: mips64-linux-gnuabi64-gcc-5 -O -Wall -static -o 
testpthsigmask-mips64 testpthsigmask.c -lpthread
  - Set environment variables for running qemu-mips64.
  - ~/inst-qemu/2.9.0/bin/qemu-mips64 testpthsigmask-mips64

  When I attach gdb to the process, I see that it is hanging inside
  'gen_intermediate_code':

  $ gdb /home/bruno/inst-qemu/2.9.0/bin/qemu-mips 9726
  ...
  Reading symbols from /home/bruno/inst-qemu/2.9.0/bin/qemu-mips...done.
  Attaching to program: /home/bruno/inst-qemu/2.9.0/bin/qemu-mips, process 9726
  ...
  (gdb) info threads
Id   Target Id Frame 
  * 1Thread 0x7f1e7e535740 (LWP 9726) "qemu-mips" __lll_lock_wait () at 
../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135
2Thread 0x7f1e7d0ad700 (LWP 9727) "qemu-mips" syscall () at 
../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
  (gdb) where
  #0  __lll_lock_wait () at ../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135
  #1  0x7f1e7d6f1dbd in __GI___pthread_mutex_lock 
(mutex=mutex@entry=0x55de1c7ff830 ) at 
../nptl/pthread_mutex_lock.c:80
  #2  0x55de1c527199 in qemu_mutex_lock (mutex=mutex@entry=0x55de1c7ff830 
)
  at /media/develdata/devel/build/qemu-2.9.0/util/qemu-thread-posix.c:60
  #3  0x55de1c435083 in tb_lock () at 
/media/develdata/devel/build/qemu-2.9.0/translate-all.c:167
  #4  cpu_restore_state (cpu=cpu@entry=0x55de1e915cb0, 
retaddr=retaddr@entry=94412445741769) at 
/media/develdata/devel/build/qemu-2.9.0/translate-all.c:350
  #5  0x55de1c4658d0 in handle_cpu_signal (old_set=0x7ffe5ffd8ea8, 
is_write=0, address=0, pc=94412445741767)
  at /media/develdata/devel/build/qemu-2.9.0/user-exec.c:124
  #6  cpu_mips_signal_handler (host_signum=host_signum@entry=11, 
pinfo=pinfo@entry=0x7ffe5ffd8eb0, puc=puc@entry=0x7ffe5ffd8d80)
  at /media/develdata/devel/build/qemu-2.9.0/user-exec.c:229
  #7  0x55de1c4803be in host_signal_handler (host_signum=11, 
info=0x7ffe5ffd8eb0, puc=0x7ffe5ffd8d80)
  at /media/develdata/devel/build/qemu-2.9.0/linux-user/signal.c:646
  #8  
  #9  __bswap_32 (__bsx=) at 
/usr/include/x86_64-linux-gnu/bits/byteswap.h:47
  #10 bswap32 (x=) at 
/media/develdata/devel/build/qemu-2.9.0/include/qemu/bswap.h:21
  #11 ldl_be_p (ptr=) at 
/media/develdata/devel/build/qemu-2.9.0/include/qemu/bswap.h:434
  #12 cpu_ldl_code (env=0x55de1e91df48, ptr=0) at 
/media/develdata/devel/build/qemu-2.9.0/include/exec/cpu_ldst_useronly_template.h:68
  #13 gen_intermediate_code (env=env@entry=0x55de1e91df48, 
tb=tb@entry=0x7f1e7b288e58)
  at /media/develdata/devel/build/qemu-2.9.0/target/mips/translate.c:19962
  #14 0x55de1c4352e6 in tb_gen_code (cpu=cpu@entry=0x55de1e915cb0, 
pc=pc@entry=0, cs_base=cs_base@entry=0, flags=flags@entry=162, 
cflags=, 
  cflags@entry=0) at 
/media/develdata/devel/build/qemu-2.9.0/translate-all.c:1295
  #15 0x55de1c436a7a in tb_find (tb_exit=0, last_tb=0x0, cpu=) at /media/develdata/devel/build/qemu-2.9.0/cpu-exec.c:365
  #16 cpu_exec (cpu=) at 
/media/develdata/devel/build/qemu-2.9.0/cpu-exec.c:673
  #17 0x55de1c466278 in cpu_loop (env=0x55de1e91df48) at 
/media/develdata/devel/build/qemu-2.9.0/linux-user/main.c:2236
  #18 0x55de1c433103 in main (argc=, argv=0x7ffe5ffd9de8, 
envp=)
  at /media/develdata/devel/build/qemu-2.9.0/linux-user/main.c:4860
  (gdb) thread 2
  [Switching to thread 2 (Thread 0x7f1e7d0ad700 (LWP 9727))]
  #0  syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
  38  ../sysdeps/unix/sysv/linux/x86_64/syscall.S: Datei oder Verzeichnis 
nicht gefunden.
  (gdb) where
  #0  syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
  #1  0x55de1c527605 in qemu_futex_wait (val=, f=) at /media/develdata/devel/build/qemu-2.9.0/include/qemu/futex.h:26
  #2  qemu_event_wait (ev=ev@entry=0x55de1e82c124 ) at 
/media/develdata/devel/build/qemu-2.9.0/util/qemu-thread-posix.c:399
  #3  0x55de1c52d41e in call_rcu_thread (opaque=) at 
/media/develdata/devel/build/qemu-2.9.0/util/rcu.c:249
  #4  0x7f1e7d6ef6ba in start_thread (arg=0x7f1e7d0ad700) at 
pthread_create.c:333
  #5  0x7f1e7d4253dd in clone () at 

Re: [Qemu-devel] [PATCH v2 02/13] hvf: add code base from Google's QEMU repository

[Izik Eidus]

On Fri, Sep 1, 2017 at 1:02 AM, Frank Yang  wrote:

> + our product manager
>
> If I understand correctly, we will need to reconsider things if I included
> any additional technology in my port. However, I didn't include any
> additional references/source in my port compared to Veertu, that was not in
> the qemu code already (e.g., hax-all/kvm-all) so I think we can just change
> the QEMU upstream port to use gplv2+.
>
> Izik, are you ok with this assessment?
>

Yes we are fine with changing everything to GPLv2+, I am just going over
the code and confirm that all copyright attributions as I saw a file that
was missing Bochs LGPLv2 copyright.



>
> On Thu, Aug 31, 2017 at 2:41 PM, Sergio Andrés Gómez del Real <
> sergio.g.delr...@gmail.com> wrote:
>
>> Hello,
>> Mr. Frank Yang was the guy at Google that introduced the HVF port to
>> Google's Android Emulator QEMU branch.
>> Frank, in this thread we are discussing the licensing issue with the HVF
>> files (their being GPL v2-only). Paolo from Red Hat was asking to Veertu
>> developer Izik Eidus if the code in Veertu derived only from QEMU, Bochs
>> and other GPLv2+ or LGPL software. Because the code at Google was itself
>> derived from Veertu, I'd imagine the same licensing terms would apply in
>> your case. Any light you can throw over this issue would be much
>> appreciated.
>>
>> Thank you.
>>
>> On Thu, Aug 31, 2017 at 4:21 PM, Paolo Bonzini 
>> wrote:
>>
>>> Il 31 ago 2017 3:54 PM, "Izik Eidus"  ha scritto:
>>>
>>> > Izik, Vincent (assuming you are the right person to contact at Google),
>>> > can you reply to Daniel and Stefan?
>>> >
>>>
>>> Hi,
>>>
>>> What I suggest is that we will send our patch' again as gpl2+ and clean
>>> the
>>> entire stuff to make sure they are falling into the right copyright
>>> category as required by QEMU.
>>>
>>>
>>> That's not necessary. Just you and Vincent replying to this thread with
>>> a "Signed-off-by" line would be enough for Sergio to use the right license
>>> in his v3 submission. Sergio already made some non-trivial changes that are
>>> needed for inclusion in QEMU from a supportability (e.g. dirty page
>>> tracking for graphics) or maintainability perspective (e.g. -cpu support),
>>> so the simplest thing to do is to retrofit the right license to his
>>> submission. You can do so if you can confirm that the code you used only
>>> came from QEMU itself, Bochs or other GPLv2+/LGPL software (and the rest
>>> was written by Veertu).
>>>
>>> Google has already contributed the HAXN accelerator, so I am moderately
>>> optimistic that they can help with HVF too.
>>>
>>> BTW, another thing that need to be integrated in order to make this stuff
>>> useful is the vmnet patch's, it is apple NAT for vms that allow guests to
>>> have networking...
>>>
>>>
>>> People can always use slirp (or tap with some more effort), so these
>>> patches are already a minimum viable feature and pretty close to being
>>> mergeable. But of course any other contribution is welcome!
>>>
>>
> That's what we do with networking in HVF for android emulator, btw; use
> slirp.
>


Yes, you guys can use our vmnet implemantion together proxy that we are
having to allow to run it without root, I think this proxy is making sense
for the android users as they won't required root, but for QEMU I think
only the vmnet implemantion will be wanted.
after this stuff will get merged we will send the vmnet stuff as GPLv2+ as
now it is GPLv2/GPLv3.

(BTW, I am pretty sure the copyright message that include GPLv2/GPLv3 and
created all this mess was taken from QEMU itself ... :))


>
>
>>
>>> Paolo
>>>
>>>
>>> (altho that it come with a trick, without certificate it
>>> will require root permission, while hypverisor framework itself can run
>>> without root)
>>>
>>> What do you guys think?
>>>
>>>
>>> >
>>> > Sergio worked on completing the QEMU port to Hypervisor.framework. The
>>> > hvf-all.c file that Daniel pointed out as v2-only is derived from
>>> kvm-all.c
>>> > and hax-all.c, and should be under v2-or-later license. The others
>>> seem to
>>> > be either original or derived from Bochs, which is LGPL, so they could
>>> be
>>> > LGPL or GPLv2+.
>>> >
>>> > Thanks,
>>> >
>>> > Paolo
>>> >
>>> >
>>> > There are benefits to having this code upstream.  If they ever want to
>>> > rebase on qemu.git there will be less work for them.
>>> >
>>> > Stefan
>>> >
>>> >
>>> >
>>>
>>>
>>>
>>
>

[Qemu-devel] [Bug 1701821] Re: floating-point operation bugs in qemu-sh4

[Bruno Haible]

With qemu-2.10, all of these tests now pass. Great!

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

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

Title:
  floating-point operation bugs in qemu-sh4

Status in QEMU:
  Fix Released

Bug description:
  When running the gnulib testsuite, I'm seeing test failures in the tests for 
libm functions
asinf
cbrtf
copysignf
coshf
expm1f
fabsf
floor
fmaf
ldexpf
logbf
round
roundf
sinhf
tanhf

  How to reproduce:
  - Using gnulib, run ./gnulib-tool --create-testdir --dir=../testdir-math 
--single-configure asinf cbrtf copysignf coshf expm1f fabsf floor fma fmaf fmal 
ldexpf logbf round roundf sinhf tanhf
  - Set environment variables for using qemu-sh4.
  - cd testdir-math; mkdir build-sh4; cd build-sh4; ./configure 
--host=sh4-linux; make; make check

  Here are the failures (from the file testdir-math/build-sh4/gltests
  /test-suite.log):

  
  FAIL: test-asinf
  

  pc=0xf6751cdc sr=0x0101 pr=0xf6758e86 fpscr=0x0008
  spc=0x ssr=0x gbr=0xf65e98e8 vbr=0x
  sgr=0x dbr=0x delayed_pc=0xf6751cd6 fpul=0x3f1a
  r0=0xf6751d88 r1=0x r2=0x0008 r3=0x
  r4=0xf6ffe21c r5=0xf6ffe230 r6=0xf6ffe2fc r7=0x
  r8=0x3f1a r9=0x3f1a r10=0x r11=0x
  r12=0xf67ab008 r13=0x r14=0x r15=0xf6ffe230
  r16=0x r17=0x r18=0x r19=0x
  r20=0x r21=0x r22=0x r23=0x
  Unhandled trap: 0x180
  FAIL test-asinf (exit status: 1)

  FAIL: test-cbrtf
  

  pc=0x00400980 sr=0x0001 pr=0x00400684 fpscr=0x0008
  spc=0x ssr=0x gbr=0xf65e98e8 vbr=0x
  sgr=0x dbr=0x delayed_pc=0x00400960 fpul=0x
  r0=0x00400ae8 r1=0x00412070 r2=0x3f1a r3=0xf6ffe2c0
  r4=0x0001 r5=0xf6ffe2f4 r6=0xf6ffe2fc r7=0x
  r8=0x00412064 r9=0x00400960 r10=0x r11=0x
  r12=0xf671dc58 r13=0x r14=0x r15=0xf6ffe21c
  r16=0x r17=0x r18=0x r19=0x
  r20=0x r21=0x r22=0x r23=0x
  Unhandled trap: 0x180
  FAIL test-cbrtf (exit status: 1)

  FAIL: test-copysignf
  

  pc=0x004004ce sr=0x0001 pr=0xf668d28c fpscr=0x0008
  spc=0x ssr=0x gbr=0xf6674678 vbr=0x
  sgr=0x dbr=0x delayed_pc=0x004004d2 fpul=0x
  r0=0x8000 r1=0x3f4d r2=0xf6674284 r3=0xf6ffe2b0
  r4=0x0001 r5=0xf6ffe2e4 r6=0xf6ffe2ec r7=0x
  r8=0x00411088 r9=0x00411084 r10=0x r11=0x
  r12=0xf67a8c58 r13=0x r14=0x r15=0xf6ffe240
  r16=0x r17=0x r18=0x r19=0x
  r20=0x r21=0x r22=0x r23=0x
  in conditional delay slot (delayed_pc=0x004004d2)
  Unhandled trap: 0x1a0
  FAIL test-copysignf (exit status: 1)

  FAIL: test-coshf
  

  pc=0xf675223a sr=0x0101 pr=0xf675223c fpscr=0x0008
  spc=0x ssr=0x gbr=0xf65e98e8 vbr=0x
  sgr=0x dbr=0x delayed_pc=0xf675231c fpul=0x3f1a
  r0=0x3f1a r1=0x3f1a r2=0x00e0 r3=0xf6ffe2c0
  r4=0x0001 r5=0xf6ffe2f4 r6=0xf6ffe2fc r7=0x
  r8=0x00400734 r9=0x r10=0x r11=0x
  r12=0xf67ab008 r13=0x r14=0x r15=0xf6ffe240
  r16=0x r17=0x r18=0x r19=0x
  r20=0x r21=0x r22=0x r23=0x
  in delay slot (delayed_pc=0xf675231c)
  Unhandled trap: 0x1a0
  FAIL test-coshf (exit status: 1)

  FAIL: test-expm1f
  =

  pc=0xf6757e08 sr=0x pr=0x004005ce fpscr=0x00081000
  spc=0x ssr=0x gbr=0xf65e98e8 vbr=0x
  sgr=0x dbr=0x delayed_pc=0xf6757dfe fpul=0x
  r0=0xf6757fb0 r1=0x1000 r2=0x0008 r3=0x3eb17218
  r4=0x0001 r5=0xf6ffe2f4 r6=0xf6ffe2fc r7=0x
  r8=0x00400514 r9=0x0064 r10=0x00400514 r11=0x
  r12=0xf67ab008 r13=0x r14=0x r15=0xf6ffe234
  r16=0x r17=0x r18=0x r19=0x
  r20=0x r21=0x r22=0x r23=0x
  Unhandled trap: 0x180
  FAIL test-expm1f (exit status: 1)

  FAIL: test-fabsf
  

  pc=0x00400504 sr=0x0001 pr=0xf660228c fpscr=0x0008
  spc=0x ssr=0x gbr=0xf65e98e8 vbr=0x
  sgr=0x dbr=0x delayed_pc=0x004004ec fpul=0x
  r0=0x00400640 r1=0x00412074 r2=0x r3=0x00412078
  r4=0x0001 r5=0xf6ffe2f4 r6=0xf6ffe2fc r7=0x0008
  r8=0x004007ac r9=0x r10=0x r11=0x
  r12=0xf671dc58 r13=0x r14=0x r15=0xf6ffe260
  r16=0x r17=0x r18=0x r19=0x
  r20=0x 

[Qemu-devel] [Bug 1701798] Re: dynamically linked binaries crash for big-endian targets

[Peter Maydell]

Can you check whether these work if you copy the QEMU and the dynamically 
linked target binary into a chroot (which does not have the x86 host ld.so or 
/etc in it) instead of using QEMU_LD_PREFIX ? There is a problem I've seen 
before where:
 1) QEMU when run with QEMU_LD_PREFIX or -L works by "first try in -L, then try 
in the host filesystem"
 2) files like /etc/ld.so.cache (and other things the dynamic linker uses) are 
not in the -L directory but are in the host
 3) the ld.so.cache format is not endian-agnostic
 4) glibc's dynamic linker code does not ignore a wrong-endian ld.so.cache but 
crashes instead

Using a chroot instead of QEMU_LD_PREFIX will work as a test of whether
this is the kind of problem you're running into. Personally I think that
(4) is a glibc bug...

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

Title:
  dynamically linked binaries crash for big-endian targets

Status in QEMU:
  New

Bug description:
  On the targets
hppa
m68k
mips
mips64
powerpc
powerpc64
s390x
sparc64
  dynamically linked binaries crash, but statically linked binaries work.
  On the targets
aarch64
alpha
armhf
powerpc64le
sh4
  both dynamically linked and statically linked binaries work.

  How to reproduce:

  1) On Ubuntu 16.04, install the packages
  g++-5-aarch64-linux-gnu
  g++-5-alpha-linux-gnu
  g++-5-arm-linux-gnueabihf
  g++-5-hppa-linux-gnu
  g++-5-m68k-linux-gnu
  g++-5-mips-linux-gnu
  g++-5-mips64-linux-gnuabi64
  g++-5-powerpc-linux-gnu
  g++-5-powerpc64-linux-gnu
  g++-5-powerpc64le-linux-gnu
  g++-5-s390x-linux-gnu
  g++-5-sh4-linux-gnu
  g++-5-sparc64-linux-gnu

  2) Install qemu 2.9.0 from source (for m68k, use the 2.7.0-m68k
  code from https://github.com/vivier/qemu-m68k.git):
  $ ../configure --prefix=/home/bruno/inst-qemu/2.9.0 
--target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,i386-softmmu,m68k-softmmu,mips-softmmu,mipsel-softmmu,mips64-softmmu,mips64el-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sh4-softmmu,sparc-softmmu,sparc64-softmmu,x86_64-softmmu,aarch64-linux-user,alpha-linux-user,arm-linux-user,hppa-linux-user,m68k-linux-user,mips-linux-user,mipsel-linux-user,mips64-linux-user,mips64el-linux-user,ppc-linux-user,ppc64-linux-user,ppc64le-linux-user,s390x-linux-user,sh4-linux-user,sparc-linux-user,sparc64-linux-user
 --disable-strip --disable-werror --enable-gtk --enable-vnc
  $ make
  $ make install

  3) Cross-compile the programs:

  $ aarch64-linux-gnu-gcc-5 -O hello.c -o hello.aarch64
  $ alpha-linux-gnu-gcc-5 -O hello.c -o hello.alpha
  $ arm-linux-gnueabihf-gcc-5 -O hello.c -o hello.armhf
  $ hppa-linux-gnu-gcc-5 -O hello.c -o hello.hppa
  $ m68k-linux-gnu-gcc-5 -O hello.c -o hello.m68k
  $ mips-linux-gnu-gcc-5 -O hello.c -o hello.mips
  $ mips64-linux-gnuabi64-gcc-5 -O hello.c -o hello.mips64
  $ powerpc-linux-gnu-gcc-5 -O hello.c -o hello.powerpc
  $ powerpc64-linux-gnu-gcc-5 -O hello.c -o hello.powerpc64
  $ powerpc64le-linux-gnu-gcc-5 -O hello.c -o hello.powerpc64le
  $ s390x-linux-gnu-gcc-5 -O hello.c -o hello.s390x
  $ sh4-linux-gnu-gcc-5 -O hello.c -o hello.sh4
  $ sparc64-linux-gnu-gcc-5 -O hello.c -o hello.sparc64

  4) Run the programs:

  * aarch64 works:
  $ QEMU_LD_PREFIX=/usr/aarch64-linux-gnu ~/inst-qemu/2.9.0/bin/qemu-aarch64 
hello.aarch64
  Hello world

  * alpha works:
  $ QEMU_LD_PREFIX=/usr/alpha-linux-gnu ~/inst-qemu/2.9.0/bin/qemu-alpha 
hello.alpha 
  Hello world

  * armhf works:
  $ QEMU_LD_PREFIX=/usr/arm-linux-gnueabihf ~/inst-qemu/2.9.0/bin/qemu-arm 
hello.armhf
  Hello world

  * powerpc64le works:
  $ QEMU_LD_PREFIX=/usr/powerpc64le-linux-gnu 
~/inst-qemu/2.9.0/bin/qemu-ppc64le hello.powerpc64le
  Hello world

  * sh4 works:
  $ QEMU_LD_PREFIX=/usr/sh4-linux-gnu ~/inst-qemu/2.9.0/bin/qemu-sh4 hello.sh4
  Hello world

  * = sparc64 does not work:
  $ QEMU_LD_PREFIX=/usr/sparc64-linux-gnu ~/inst-qemu/2.9.0/bin/qemu-sparc64 
hello.sparc64
  Segmentation fault (core dumped)

  When I copy the file to a machine with `uname -srm` = "Linux 4.5.0-2-sparc64 
sparc64",
  it works:
  $ ./hello.sparc64
  Hello world

  When I copy the file and its execution environment /usr/sparc64-linux-gnu to 
the
  same machine and run the binary in a chroot environment:
  # /bin/hello.sparc64 
  Hello world

  * = mips does not work:
  $ QEMU_LD_PREFIX=/usr/mips-linux-gnu ~/inst-qemu/2.9.0/bin/qemu-mips 
hello.mips
  qemu: uncaught target signal 11 (Segmentation fault) - core dumped

  When I copy the file to a machine with `uname -srm` = "Linux 
3.16.0-4-4kc-malta mips",
  it works:
  $ ./hello.mips
  Hello world

  When I copy the file and its execution environment /usr/mips-linux-gnu to the
  same machine and run the binary in a chroot environment:
  # /bin/hello.mips 
  Hello world

  * = mips64 does not work:
  $ QEMU_LD_PREFIX=/usr/mips64-linux-gnuabi64 ~/inst-qemu/2.9.0/bin/qemu-mips64 

[Qemu-devel] [Bug 1701808] Re: stack smashing in or after recvmsg system call in aarch64 user mode

[Bruno Haible]

The behaviour in qemu-2.10 is the same as in qemu-2.9.

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

Title:
  stack smashing in or after recvmsg system call in aarch64 user mode

Status in QEMU:
  New

Bug description:
  A program that invokes recvmsg aborts with "*** stack smashing
  detected ***" when run in qemu-aarch64 (user mode), but works fine
  when running on native aarch64 hardware.

  How to reproduce:
  $ aarch64-linux-gnu-gcc-5 -O -Wall 
/media/develdata/devel/qemu-bug/testpassfd.c -static -DEXTRA_SPACE=0
  $ QEMU_LD_PREFIX=/usr/aarch64-linux-gnu ~/inst-qemu/2.9.0/bin/qemu-aarch64 
./a.out
  *** stack smashing detected ***: ./a.out terminated
  qemu: uncaught target signal 6 (Aborted) - core dumped

  On native aarch64 hardware:
  $ ./a.out 
  $ echo $?
  0

  The parameter EXTRA_SPACE can be used to add additional space to the
  array that receives the recvmsg data. With -DEXTRA_SPACE=9 (or
  larger), the program runs fine. Which suggests that recvmsg is storing
  up to 9 bytes more than allowed in memory.

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

[Qemu-devel] [Bug 1701798] Re: dynamically linked binaries crash for big-endian targets

[Bruno Haible]

The behaviour in qemu-2.10 is nearly the same as in qemu-2.9. The only
difference is a different kind of crash for m68k:

$ QEMU_LD_PREFIX=/usr/m68k-linux-gnu QEMU_CPU=m68020 
~/inst-qemu/2.9.0/bin/qemu-m68k hello.m68k
qemu: uncaught target signal 4 (Illegal instruction) - core dumped

$ QEMU_LD_PREFIX=/usr/m68k-linux-gnu QEMU_CPU=m68020 
~/inst-qemu/2.10.0/bin/qemu-m68k hello.m68k
qemu: uncaught target signal 11 (Segmentation fault) - core dumped

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

Title:
  dynamically linked binaries crash for big-endian targets

Status in QEMU:
  New

Bug description:
  On the targets
hppa
m68k
mips
mips64
powerpc
powerpc64
s390x
sparc64
  dynamically linked binaries crash, but statically linked binaries work.
  On the targets
aarch64
alpha
armhf
powerpc64le
sh4
  both dynamically linked and statically linked binaries work.

  How to reproduce:

  1) On Ubuntu 16.04, install the packages
  g++-5-aarch64-linux-gnu
  g++-5-alpha-linux-gnu
  g++-5-arm-linux-gnueabihf
  g++-5-hppa-linux-gnu
  g++-5-m68k-linux-gnu
  g++-5-mips-linux-gnu
  g++-5-mips64-linux-gnuabi64
  g++-5-powerpc-linux-gnu
  g++-5-powerpc64-linux-gnu
  g++-5-powerpc64le-linux-gnu
  g++-5-s390x-linux-gnu
  g++-5-sh4-linux-gnu
  g++-5-sparc64-linux-gnu

  2) Install qemu 2.9.0 from source (for m68k, use the 2.7.0-m68k
  code from https://github.com/vivier/qemu-m68k.git):
  $ ../configure --prefix=/home/bruno/inst-qemu/2.9.0 
--target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,i386-softmmu,m68k-softmmu,mips-softmmu,mipsel-softmmu,mips64-softmmu,mips64el-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sh4-softmmu,sparc-softmmu,sparc64-softmmu,x86_64-softmmu,aarch64-linux-user,alpha-linux-user,arm-linux-user,hppa-linux-user,m68k-linux-user,mips-linux-user,mipsel-linux-user,mips64-linux-user,mips64el-linux-user,ppc-linux-user,ppc64-linux-user,ppc64le-linux-user,s390x-linux-user,sh4-linux-user,sparc-linux-user,sparc64-linux-user
 --disable-strip --disable-werror --enable-gtk --enable-vnc
  $ make
  $ make install

  3) Cross-compile the programs:

  $ aarch64-linux-gnu-gcc-5 -O hello.c -o hello.aarch64
  $ alpha-linux-gnu-gcc-5 -O hello.c -o hello.alpha
  $ arm-linux-gnueabihf-gcc-5 -O hello.c -o hello.armhf
  $ hppa-linux-gnu-gcc-5 -O hello.c -o hello.hppa
  $ m68k-linux-gnu-gcc-5 -O hello.c -o hello.m68k
  $ mips-linux-gnu-gcc-5 -O hello.c -o hello.mips
  $ mips64-linux-gnuabi64-gcc-5 -O hello.c -o hello.mips64
  $ powerpc-linux-gnu-gcc-5 -O hello.c -o hello.powerpc
  $ powerpc64-linux-gnu-gcc-5 -O hello.c -o hello.powerpc64
  $ powerpc64le-linux-gnu-gcc-5 -O hello.c -o hello.powerpc64le
  $ s390x-linux-gnu-gcc-5 -O hello.c -o hello.s390x
  $ sh4-linux-gnu-gcc-5 -O hello.c -o hello.sh4
  $ sparc64-linux-gnu-gcc-5 -O hello.c -o hello.sparc64

  4) Run the programs:

  * aarch64 works:
  $ QEMU_LD_PREFIX=/usr/aarch64-linux-gnu ~/inst-qemu/2.9.0/bin/qemu-aarch64 
hello.aarch64
  Hello world

  * alpha works:
  $ QEMU_LD_PREFIX=/usr/alpha-linux-gnu ~/inst-qemu/2.9.0/bin/qemu-alpha 
hello.alpha 
  Hello world

  * armhf works:
  $ QEMU_LD_PREFIX=/usr/arm-linux-gnueabihf ~/inst-qemu/2.9.0/bin/qemu-arm 
hello.armhf
  Hello world

  * powerpc64le works:
  $ QEMU_LD_PREFIX=/usr/powerpc64le-linux-gnu 
~/inst-qemu/2.9.0/bin/qemu-ppc64le hello.powerpc64le
  Hello world

  * sh4 works:
  $ QEMU_LD_PREFIX=/usr/sh4-linux-gnu ~/inst-qemu/2.9.0/bin/qemu-sh4 hello.sh4
  Hello world

  * = sparc64 does not work:
  $ QEMU_LD_PREFIX=/usr/sparc64-linux-gnu ~/inst-qemu/2.9.0/bin/qemu-sparc64 
hello.sparc64
  Segmentation fault (core dumped)

  When I copy the file to a machine with `uname -srm` = "Linux 4.5.0-2-sparc64 
sparc64",
  it works:
  $ ./hello.sparc64
  Hello world

  When I copy the file and its execution environment /usr/sparc64-linux-gnu to 
the
  same machine and run the binary in a chroot environment:
  # /bin/hello.sparc64 
  Hello world

  * = mips does not work:
  $ QEMU_LD_PREFIX=/usr/mips-linux-gnu ~/inst-qemu/2.9.0/bin/qemu-mips 
hello.mips
  qemu: uncaught target signal 11 (Segmentation fault) - core dumped

  When I copy the file to a machine with `uname -srm` = "Linux 
3.16.0-4-4kc-malta mips",
  it works:
  $ ./hello.mips
  Hello world

  When I copy the file and its execution environment /usr/mips-linux-gnu to the
  same machine and run the binary in a chroot environment:
  # /bin/hello.mips 
  Hello world

  * = mips64 does not work:
  $ QEMU_LD_PREFIX=/usr/mips64-linux-gnuabi64 ~/inst-qemu/2.9.0/bin/qemu-mips64 
hello.mips64
  qemu: uncaught target signal 11 (Segmentation fault) - core dumped

  When I copy the file to a machine with `uname -srm` = "Linux 
3.16.0-4-5kc-malta mips64",
  it works:
  $ ./hello.mips64
  Hello world

  * = powerpc does not work:
  $ QEMU_LD_PREFIX=/usr/powerpc-linux-gnu ~/inst-qemu/2.9.0/bin/qemu-ppc 

Re: [Qemu-devel] [PATCH v2 02/13] hvf: add code base from Google's QEMU repository

[Sergio Andrés Gómez del Real]

Hello,
Mr. Frank Yang was the guy at Google that introduced the HVF port to
Google's Android Emulator QEMU branch.
Frank, in this thread we are discussing the licensing issue with the HVF
files (their being GPL v2-only). Paolo from Red Hat was asking to Veertu
developer Izik Eidus if the code in Veertu derived only from QEMU, Bochs
and other GPLv2+ or LGPL software. Because the code at Google was itself
derived from Veertu, I'd imagine the same licensing terms would apply in
your case. Any light you can throw over this issue would be much
appreciated.

Thank you.

On Thu, Aug 31, 2017 at 4:21 PM, Paolo Bonzini  wrote:

> Il 31 ago 2017 3:54 PM, "Izik Eidus"  ha scritto:
>
> > Izik, Vincent (assuming you are the right person to contact at Google),
> > can you reply to Daniel and Stefan?
> >
>
> Hi,
>
> What I suggest is that we will send our patch' again as gpl2+ and clean the
> entire stuff to make sure they are falling into the right copyright
> category as required by QEMU.
>
>
> That's not necessary. Just you and Vincent replying to this thread with a
> "Signed-off-by" line would be enough for Sergio to use the right license in
> his v3 submission. Sergio already made some non-trivial changes that are
> needed for inclusion in QEMU from a supportability (e.g. dirty page
> tracking for graphics) or maintainability perspective (e.g. -cpu support),
> so the simplest thing to do is to retrofit the right license to his
> submission. You can do so if you can confirm that the code you used only
> came from QEMU itself, Bochs or other GPLv2+/LGPL software (and the rest
> was written by Veertu).
>
> Google has already contributed the HAXN accelerator, so I am moderately
> optimistic that they can help with HVF too.
>
> BTW, another thing that need to be integrated in order to make this stuff
> useful is the vmnet patch's, it is apple NAT for vms that allow guests to
> have networking...
>
>
> People can always use slirp (or tap with some more effort), so these
> patches are already a minimum viable feature and pretty close to being
> mergeable. But of course any other contribution is welcome!
>
> Paolo
>
>
> (altho that it come with a trick, without certificate it
> will require root permission, while hypverisor framework itself can run
> without root)
>
> What do you guys think?
>
>
> >
> > Sergio worked on completing the QEMU port to Hypervisor.framework. The
> > hvf-all.c file that Daniel pointed out as v2-only is derived from
> kvm-all.c
> > and hax-all.c, and should be under v2-or-later license. The others seem
> to
> > be either original or derived from Bochs, which is LGPL, so they could be
> > LGPL or GPLv2+.
> >
> > Thanks,
> >
> > Paolo
> >
> >
> > There are benefits to having this code upstream.  If they ever want to
> > rebase on qemu.git there will be less work for them.
> >
> > Stefan
> >
> >
> >
>
>
>

Re: [Qemu-devel] [PATCH v2 02/13] hvf: add code base from Google's QEMU repository

[Izik Eidus]

On Fri, Sep 1, 2017 at 12:21 AM, Paolo Bonzini  wrote:

> Il 31 ago 2017 3:54 PM, "Izik Eidus"  ha scritto:
>
> > Izik, Vincent (assuming you are the right person to contact at Google),
> > can you reply to Daniel and Stefan?
> >
>
> Hi,
>
> What I suggest is that we will send our patch' again as gpl2+ and clean the
> entire stuff to make sure they are falling into the right copyright
> category as required by QEMU.
>
>
> That's not necessary. Just you and Vincent replying to this thread with a
> "Signed-off-by" line would be enough for Sergio to use the right license in
> his v3 submission. Sergio already made some non-trivial changes that are
> needed for inclusion in QEMU from a supportability (e.g. dirty page
> tracking for graphics) or maintainability perspective (e.g. -cpu support),
> so the simplest thing to do is to retrofit the right license to his
> submission. You can do so if you can confirm that the code you used only
> came from QEMU itself, Bochs or other GPLv2+/LGPL software (and the rest
> was written by Veertu).
>

Hi,

Sure fine with us, let me go over all the code and see that all copyright
that are needed are there, and then you can relicense all our code to
GPLv2+, I think I saw a a file that was missing Bochs copyright in it, so I
want to make sure that I fix this before and that all others are fine.

Thanks.


>
> Google has already contributed the HAXN accelerator, so I am moderately
> optimistic that they can help with HVF too.
>
> BTW, another thing that need to be integrated in order to make this stuff
> useful is the vmnet patch's, it is apple NAT for vms that allow guests to
> have networking...
>
>
> People can always use slirp (or tap with some more effort), so these
> patches are already a minimum viable feature and pretty close to being
> mergeable. But of course any other contribution is welcome!
>
> Paolo
>
>
> (altho that it come with a trick, without certificate it
> will require root permission, while hypverisor framework itself can run
> without root)
>
> What do you guys think?
>
>
> >
> > Sergio worked on completing the QEMU port to Hypervisor.framework. The
> > hvf-all.c file that Daniel pointed out as v2-only is derived from
> kvm-all.c
> > and hax-all.c, and should be under v2-or-later license. The others seem
> to
> > be either original or derived from Bochs, which is LGPL, so they could be
> > LGPL or GPLv2+.
> >
> > Thanks,
> >
> > Paolo
> >
> >
> > There are benefits to having this code upstream.  If they ever want to
> > rebase on qemu.git there will be less work for them.
> >
> > Stefan
> >
> >
> >
>
>
>

Re: [Qemu-devel] [PATCH v5 1/3] hw/acpi-build: Fix SRAT memory building in case of node 0 without RAM

[Eduardo Habkost]

On Thu, Aug 31, 2017 at 08:04:26PM +0800, Dou Liyang wrote:
> From: Eduardo Habkost 
> 
> Currently, Using the fisrt node without memory on the machine makes
> QEMU unhappy. With this example command line:
>   ... \
>   -m 1024M,slots=4,maxmem=32G \
>   -numa node,nodeid=0 \
>   -numa node,mem=1024M,nodeid=1 \
>   -numa node,nodeid=2 \
>   -numa node,nodeid=3 \
> Guest reports "No NUMA configuration found" and the NUMA topology is
> wrong.
> 
> This is because when QEMU builds ACPI SRAT, it regards node 0 as the
> default node to deal with the memory hole(640K-1M). this means the
> node0 must have some memory(>1M), but, actually it can have no
> memory.
> 
> Fix this problem by  cut out the 640K hole in the same way the PCI
> 4G hole does. Also do some cleanup.
> 
> Signed-off-by: Eduardo Habkost 
> Signed-off-by: Dou Liyang 
> ---
>  hw/i386/acpi-build.c | 30 +++---
>  1 file changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 98dd424..48525a1 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2318,6 +2318,9 @@ build_tpm2(GArray *table_data, BIOSLinker *linker)
>   (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL);
>  }
>  
> +#define HOLE_640K_START  (640 * 1024)
> +#define HOLE_640K_END   (1024 * 1024)
> +
>  static void
>  build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>  {
> @@ -2373,17 +2376,30 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
> MachineState *machine)
>  next_base = 0;
>  numa_start = table_data->len;
>  
> -numamem = acpi_data_push(table_data, sizeof *numamem);
> -build_srat_memory(numamem, 0, 640 * 1024, 0, MEM_AFFINITY_ENABLED);
> -next_base = 1024 * 1024;
>  for (i = 1; i < pcms->numa_nodes + 1; ++i) {
>  mem_base = next_base;
>  mem_len = pcms->node_mem[i - 1];
> -if (i == 1) {
> -mem_len -= 1024 * 1024;
> -}
>  next_base = mem_base + mem_len;
>  
> +/* Cut out the 640K hole */
> +if (mem_base <= HOLE_640K_START &&
> +next_base > HOLE_640K_START) {
> +mem_len -= next_base - HOLE_640K_START;
> +if (mem_len > 0) {
> +numamem = acpi_data_push(table_data, sizeof *numamem);
> +build_srat_memory(numamem, mem_base, mem_len, i - 1,
> +  MEM_AFFINITY_ENABLED);
> +}
> +
> +/* Check for the rare case: 640K < RAM < 1M */
> +if (next_base <= HOLE_640K_END) {
> +next_base = HOLE_640K_END;
> +continue;
> +}
> +mem_base = HOLE_640K_END;
> +mem_len = next_base - HOLE_640K_END;
> +}
> +
>  /* Cut out the ACPI_PCI hole */
>  if (mem_base <= pcms->below_4g_mem_size &&
>  next_base > pcms->below_4g_mem_size) {
> @@ -2395,7 +2411,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
> MachineState *machine)
>  }
>  mem_base = 1ULL << 32;
>  mem_len = next_base - pcms->below_4g_mem_size;
> -next_base += (1ULL << 32) - pcms->below_4g_mem_size;
> +next_base = mem_base + mem_len;

Is this extra change intentional?

I find the code more readable with it, but it should go in a
separate patch because it is unrelated to the bug fix.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 02/13] hvf: add code base from Google's QEMU repository

[Paolo Bonzini]

Il 31 ago 2017 3:54 PM, "Izik Eidus"  ha scritto:

> Izik, Vincent (assuming you are the right person to contact at Google),
> can you reply to Daniel and Stefan?
>

Hi,

What I suggest is that we will send our patch' again as gpl2+ and clean the
entire stuff to make sure they are falling into the right copyright
category as required by QEMU.


That's not necessary. Just you and Vincent replying to this thread with a
"Signed-off-by" line would be enough for Sergio to use the right license in
his v3 submission. Sergio already made some non-trivial changes that are
needed for inclusion in QEMU from a supportability (e.g. dirty page
tracking for graphics) or maintainability perspective (e.g. -cpu support),
so the simplest thing to do is to retrofit the right license to his
submission. You can do so if you can confirm that the code you used only
came from QEMU itself, Bochs or other GPLv2+/LGPL software (and the rest
was written by Veertu).

Google has already contributed the HAXN accelerator, so I am moderately
optimistic that they can help with HVF too.

BTW, another thing that need to be integrated in order to make this stuff
useful is the vmnet patch's, it is apple NAT for vms that allow guests to
have networking...


People can always use slirp (or tap with some more effort), so these
patches are already a minimum viable feature and pretty close to being
mergeable. But of course any other contribution is welcome!

Paolo


(altho that it come with a trick, without certificate it
will require root permission, while hypverisor framework itself can run
without root)

What do you guys think?


>
> Sergio worked on completing the QEMU port to Hypervisor.framework. The
> hvf-all.c file that Daniel pointed out as v2-only is derived from
kvm-all.c
> and hax-all.c, and should be under v2-or-later license. The others seem to
> be either original or derived from Bochs, which is LGPL, so they could be
> LGPL or GPLv2+.
>
> Thanks,
>
> Paolo
>
>
> There are benefits to having this code upstream.  If they ever want to
> rebase on qemu.git there will be less work for them.
>
> Stefan
>
>
>

[Qemu-devel] [Bug 997631] Re: Windows 2008R2 very slow cold boot when 4 CPUs

[Thomas Huth]

Triaging old bug tickets... can you still reproduce this issue with the
latest version of QEMU? Or could we close this ticket nowadays?

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  Windows 2008R2 very slow cold boot when 4 CPUs

Status in QEMU:
  Incomplete

Bug description:
  Hi,

  well, I'm in a similar boat as the one in #992067. But regardless any 
memory-settings.
  It takes "ages" in a cold-boot Windows 2008R2 with qemu-1.0.1, qemu-1.0.50 
and latest-n-greatest from today ( 1.0.50 /qemu-1b3e76e ). It eats up 400% 
host-cpu-load until login-prompt is shown on the console.

  Meanwhile I tried couple of settings with "-cpu features (hv_spinlocks), 
hv_relaxed and hv_vapic. ".
  Due to some Clock-glitches I start qemu-system-x86_64 with "-no-hpet".

  With 2 processors the system is up after 2 minutes, with 4 procs
  almost 10 minutes... After a reset ( warmstart) the 4 proc-system is
  up after a couple of 20 secs.

  Hints welcome, though once started, the system seems to operate
  "normally".

  Thnx in@vance,

  Oliver.

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

[Qemu-devel] [Bug 1714331] [NEW] Virtual machines not working anymore on 2.10

[Mary Sherman]

Public bug reported:

Using 2.10, my virtual machine(s) don't work anymore. This happens 100%
of the times.

-

I use QEMU compiling it from source, on Ubuntu 16.04 amd64. This is the
configure command:

configure --target-list=x86_64-softmmu --enable-debug --enable-gtk
--enable-spice --audio-drv-list=pa

I have one virtual disk, with a Windows 10 64-bit, which I launch in two
different ways; both work perfectly on 2.9 (and used to do on 2.8, but I
haven't used it for a long time).

This is the first way:

qemu-system-x86_64
  -drive if=pflash,format=raw,readonly,file=/path/to/OVMF_CODE.fd
  -drive if=pflash,format=raw,file=/tmp/OVMF_VARS.fd.tmp
  -enable-kvm
  -machine q35,accel=kvm,mem-merge=off
  -cpu 
host,kvm=off,hv_vendor_id=vgaptrocks,hv_relaxed,hv_spinlocks=0x1fff,hv_vapic,hv_time
  -smp 4,cores=4,sockets=1,threads=1
  -m 4096
  -display gtk
  -vga qxl
  -rtc base=localtime
  -serial none
  -parallel none
  -usb
  -device usb-host,vendorid=0x,productid=0x
  -device usb-host,vendorid=0x,productid=0x
  -device usb-host,vendorid=0x,productid=0x
  -device usb-host,vendorid=0x,productid=0x
  -device virtio-scsi-pci,id=scsi
  -drive 
file=/path/to/image-diff.img,id=hdd1,format=qcow2,if=none,cache=writeback
  -device scsi-hd,drive=hdd1
  -net nic,model=virtio
  -net user

On QEMU 2.10, I get the `Recovery - Your PC/Device needs to be repaired`
windows screen; on 2.9, it boots regularly.

This is the second way:

qemu-system-x86_64
  -drive if=pflash,format=raw,readonly,file=/path/to/OVMF_CODE.fd
  -drive if=pflash,format=raw,file=/tmp/OVMF_VARS.fd.tmp
  -enable-kvm
  -machine q35,accel=kvm,mem-merge=off
  -cpu 
host,kvm=off,hv_vendor_id=vgaptrocks,hv_relaxed,hv_spinlocks=0x1fff,hv_vapic,hv_time
  -smp 4,cores=4,sockets=1,threads=1
  -m 10240
  -vga none
  -rtc base=localtime
  -serial none
  -parallel none
  -usb
  -device vfio-pci,host=01:00.0,multifunction=on
  -device vfio-pci,host=01:00.1
  -device usb-host,vendorid=0x,productid=0x
  -device usb-host,vendorid=0x,productid=0x
  -device usb-host,vendorid=0x,productid=0x
  -device usb-host,vendorid=0x,productid=0x
  -device usb-host,vendorid=0x,productid=0x
  -device usb-host,vendorid=0x,productid=0x
  -device virtio-scsi-pci,id=scsi
  -drive 
file=/path/to/image-diff.img,id=hdd1,format=qcow2,if=none,cache=writeback
  -device scsi-hd,drive=hdd1
  -net nic,model=virtio
  -net user

On QEMU 2.10, I get the debug window on the linux monitor, and blank screen on 
VFIO one (no BIOS screen at all); after 10/20 seconds, QEMU crashes without any 
message.
On 2.9, this works perfectly.

-

I am able to perform a git bisect, if that helps, but if this is the
case, I'd need this issue to be reviewed, since bisecting is going to
take me a lot of time.

** 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/1714331

Title:
  Virtual machines not working anymore on 2.10

Status in QEMU:
  New

Bug description:
  Using 2.10, my virtual machine(s) don't work anymore. This happens
  100% of the times.

  -

  I use QEMU compiling it from source, on Ubuntu 16.04 amd64. This is
  the configure command:

  configure --target-list=x86_64-softmmu --enable-debug --enable-gtk
  --enable-spice --audio-drv-list=pa

  I have one virtual disk, with a Windows 10 64-bit, which I launch in
  two different ways; both work perfectly on 2.9 (and used to do on 2.8,
  but I haven't used it for a long time).

  This is the first way:

  qemu-system-x86_64
-drive if=pflash,format=raw,readonly,file=/path/to/OVMF_CODE.fd
-drive if=pflash,format=raw,file=/tmp/OVMF_VARS.fd.tmp
-enable-kvm
-machine q35,accel=kvm,mem-merge=off
-cpu 
host,kvm=off,hv_vendor_id=vgaptrocks,hv_relaxed,hv_spinlocks=0x1fff,hv_vapic,hv_time
-smp 4,cores=4,sockets=1,threads=1
-m 4096
-display gtk
-vga qxl
-rtc base=localtime
-serial none
-parallel none
-usb
-device usb-host,vendorid=0x,productid=0x
-device usb-host,vendorid=0x,productid=0x
-device usb-host,vendorid=0x,productid=0x
-device usb-host,vendorid=0x,productid=0x
-device virtio-scsi-pci,id=scsi
-drive 
file=/path/to/image-diff.img,id=hdd1,format=qcow2,if=none,cache=writeback
-device scsi-hd,drive=hdd1
-net nic,model=virtio
-net user

  On QEMU 2.10, I get the `Recovery - Your PC/Device needs to be
  repaired` windows screen; on 2.9, it boots regularly.

  This is the second way:

  qemu-system-x86_64

Re: [Qemu-devel] [Qemu-stable] [PATCH 00/79] Patch Round-up for stable 2.9.1, freeze on 2017-09-04

[Peter Lieven]

Am 29.08.2017 um 02:13 schrieb Michael Roth:
> Hi everyone,
>
> The following new patches are queued for QEMU stable v2.9.1:
>
>   https://github.com/mdroth/qemu/commits/stable-2.9-staging
>
> The release is planned for 2017-09-07:
>
>   http://wiki.qemu.org/Planning/2.9
>
> Please respond here or CC qemu-sta...@nongnu.org on any patches you
> think should be included in the release.

I also have:

452589b vl.c/exit: pause cpus before closing block devices

Peter

Re: [Qemu-devel] [PATCH 00/79] Patch Round-up for stable 2.9.1, freeze on 2017-09-04

[Michael Roth]

Quoting Michael Roth (2017-08-28 19:13:35)
> Hi everyone,
> 
> The following new patches are queued for QEMU stable v2.9.1:
> 
>   https://github.com/mdroth/qemu/commits/stable-2.9-staging

Thank you for the recommendations. Branch updated with the following
additional patches:

 s390-ccw: Fix alignment for CCW1
 vnc: Set default kbd delay to 10ms
 qemu-nbd: Ignore SIGPIPE
 usb-redir: fix stack overflow in usbredir_log_data
 megasas: do not read SCSI req parameters more than once from frame
 megasas: do not read command more than once from frame
 megasas: do not read DCMD opcode more than once from frame
 megasas: do not read iovec count more than once from frame
 megasas: do not read sense length more than once from frame
 9pfs: local: forbid client access to metadata (CVE-2017-7493)
 scsi: avoid an off-by-one error in megasas_mmio_write
 audio: release capture buffers
 vmw_pvscsi: check message ring page count at initialisation
 hw/ppc/spapr_iommu: Fix crash when removing the "spapr-tce-table" device
 hw/ppc/spapr_rtc: Mark the RTC device with user_creatable = false
 qdev: Replace cannot_instantiate_with_device_add_yet with !user_creatable
 fix qemu-system-unicore32 crashing when calling without -kernel
 hw/s390x/ipl: Fix crash with virtio-scsi-pci device
 slirp: fix clearing ifq_so from pending packets
 slirp: tftp, copy sockaddr_size
 monitor: Check whether TCG is enabled before running the "info jit" code
 target-s390x: Mask the SIGP order_code to 8bit.

Re: [Qemu-devel] [PATCH 00/79] Patch Round-up for stable 2.9.1, freeze on 2017-09-04

[Michael Roth]

Quoting Peter Maydell (2017-08-31 12:07:08)
> On 31 August 2017 at 17:42, Michael Roth  wrote:
> > Quoting Thomas Huth (2017-08-28 21:18:20)
> >> Not sure, but maybe the following patch should be included, too, since
> >> there were some bogus files in the old version of the U-Boot sources:
> >>
> >> 73663d71ef2bab201475d58e - PPC: E500: Update u-boot to v2017.07
> >
> > Do you have more background on any issues caused by these bogus files?
> > As it stands I think I would opt not to update unless there are specific
> > user-visible bugs we're trying to address which warrant the risk of any
> > regressions which might get pulled in in the process.
> 
> These are the relevant threads:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddiscuss_2017-2D07_msg5.html=DwIBaQ=jf_iaSHvJObTbx-siA1ZOg=sThPI1c0u5x-3sg5Nw8wNqjg_5Z5xLzfPGC18E94zn8=QzqXTgvEiqKKSlIJgVfNCEiYXPQ5oVFxHFdUcgtf_L8=4BVFXlpHawdLmHljZCHMSNEVaj8JzUuNJw6HgKZzvn0=
>  
> and
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2017-2D07_msg02956.html=DwIBaQ=jf_iaSHvJObTbx-siA1ZOg=sThPI1c0u5x-3sg5Nw8wNqjg_5Z5xLzfPGC18E94zn8=QzqXTgvEiqKKSlIJgVfNCEiYXPQ5oVFxHFdUcgtf_L8=iO72EaulRM4jy_9QBnIcqH5K_hIWtAmBOMqy6QORs2M=
>  
> 
> The summary is
> (1) one of the u-boot source files which is distributed as part
> of the QEMU tarball has a comment which makes it a bit unclear
> whether it's something that's redistributable (the source file
> isn't actually used in the u-boot target we care about)
> (2) the u-boot binary blob we were shipping doesn't correspond
> to the sources we were shipping
> 
> and we fixed those in master by updating the blob and the
> submodule to the most recent u-boot.
> 
> I guess the low-risk fix for the stable branch would be to
> update the u-boot submodule to 79c884d7e4 as suggested in
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2017-2D07_msg03174.html=DwIBaQ=jf_iaSHvJObTbx-siA1ZOg=sThPI1c0u5x-3sg5Nw8wNqjg_5Z5xLzfPGC18E94zn8=QzqXTgvEiqKKSlIJgVfNCEiYXPQ5oVFxHFdUcgtf_L8=WY3VTHQGDP63Rw7hykVtVSbAqb8db-of8rkUG3hrlUg=
>  
> which would bring the distributed sources into line with
> the binary blob in stable, so no need to change the
> blob we're distributing. I think it makes sense to do that
> for stable.

Thanks for the background/suggestion, I think I'll take this
approach.

> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] S390 bios breaks in qemu 2.10.rc3

[Michael Roth]

Quoting Thomas Huth (2017-08-29 04:35:22)
> On 28.08.2017 09:18, Christian Borntraeger wrote:
> > 
> > 
> > On 08/25/2017 10:29 AM, Cornelia Huck wrote:
> >> On Fri, 25 Aug 2017 10:21:58 +0200
> >> Christian Borntraeger  wrote:
> >>
> >>> On 08/25/2017 09:20 AM, Cornelia Huck wrote:
> >>
>  OK, to recap:
> 
>  - the current pre-built bios seems fine
>  - rebuilding the bios may yield a version that fails on some systems
>    (different compiler?)
>  - adding aligned(8) looks like the right thing to do
>  - it seems to fix the problem, but on at least one system something
>    still seems off (under investigation)  
> >>>
> >>> Yes. I am out of office today, so for any aligned(8) patch
> >>> Acked-by: Christian Borntraeger 
> >>> even for 2.10.
> >>
> >> I fear the 2.10 train has already left the station, but any aligned(8)
> >> patch should be cc:stable.
> > 
> > I think this could be a topic for QEMU summit. Our process of not allowing
> > fixes in rcx without requiring an rc(x+1) seems a bit odd. The Linux kernel
> > style (there are fixes between the last rc and release) seems more balanced
> > as long as we establish some safety nets.
> 
> This sounds like a good idea to me, yes. And maybe we could also ease
> the situation a little bit by providing the first stable .1 release
> already two or three weeks after the .0 release [*] ? Then these "we are
> not 100% sure whether this is a severe blocker or not" patches could
> simply be provided to the public with that .1 release instead of
> blocking the QEMU master branch in freeze state...

I can give it a try for 2.10.1.

At least initially, I would prefer to not have there be a set expectation
that there will be a quick .1 release though, but rather anything tagged
for-2.10, for instance, that doesn't make it in, will then fall into
the "expedited 2.10.1" bucket, and for that to be the *only* way to get
into that bucket (we'd still do the normal round-up and pull in any
"Cc: qemu-sta...@nongnu.org" patches at that point though).

This would help ensure we don't steal focus from the .0 releases and allow
those "is this a blocker?" discussions to happen in the proper context.

> 
>  Thomas
> 
> 
> [*] I know that means more additional work for Michael - sorry for that
> ... but at least we should talk about this, I think. Maybe someone else
> could also help with the releases if it's too much work for one person?
> 

Re: [Qemu-devel] [PATCH v1 1/1] target/xtensa: Use the pre-defined MEMTXATTRS_UNSPECIFIED macro

[Alistair Francis]

On Wed, Aug 30, 2017 at 3:12 PM, Peter Maydell  wrote:
> On 30 August 2017 at 19:02, Alistair Francis
>  wrote:
>> Instead of using the hardcoded (MemTxAttrs){0} for no memory attributes
>> let's use the already defined MEMTXATTRS_UNSPECIFIED macro instead.
>>
>> Signed-off-by: Alistair Francis 
>> ---
>>
>>  target/xtensa/op_helper.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/xtensa/op_helper.c b/target/xtensa/op_helper.c
>> index 519fbeddd6..3d990c0caa 100644
>> --- a/target/xtensa/op_helper.c
>> +++ b/target/xtensa/op_helper.c
>> @@ -1025,11 +1025,11 @@ void HELPER(ule_s)(CPUXtensaState *env, uint32_t br, 
>> float32 a, float32 b)
>>  uint32_t HELPER(rer)(CPUXtensaState *env, uint32_t addr)
>>  {
>>  return address_space_ldl(env->address_space_er, addr,
>> - (MemTxAttrs){0}, NULL);
>> + MEMTXATTRS_UNSPECIFIED, NULL);
>>  }
>>
>>  void HELPER(wer)(CPUXtensaState *env, uint32_t data, uint32_t addr)
>>  {
>>  address_space_stl(env->address_space_er, addr, data,
>> -  (MemTxAttrs){0}, NULL);
>> +  MEMTXATTRS_UNSPECIFIED, NULL);
>>  }
>
> Might be worth noting in the commit that this is technically
> a change of behaviour, because MEMTXATTRS_UNSPECIFIED
> sets the 'unspecified' field to 1 whereas {0} doesn't.
> I don't think anything actually checks that field, though.

Good point, I have added something to the commit message to indicate
that. It'll be included in v2.

Thanks,
Alistair

>
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH 00/79] Patch Round-up for stable 2.9.1, freeze on 2017-09-04

[Peter Maydell]

On 31 August 2017 at 17:42, Michael Roth  wrote:
> Quoting Thomas Huth (2017-08-28 21:18:20)
>> Not sure, but maybe the following patch should be included, too, since
>> there were some bogus files in the old version of the U-Boot sources:
>>
>> 73663d71ef2bab201475d58e - PPC: E500: Update u-boot to v2017.07
>
> Do you have more background on any issues caused by these bogus files?
> As it stands I think I would opt not to update unless there are specific
> user-visible bugs we're trying to address which warrant the risk of any
> regressions which might get pulled in in the process.

These are the relevant threads:
https://lists.gnu.org/archive/html/qemu-discuss/2017-07/msg5.html
and
https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg02956.html

The summary is
(1) one of the u-boot source files which is distributed as part
of the QEMU tarball has a comment which makes it a bit unclear
whether it's something that's redistributable (the source file
isn't actually used in the u-boot target we care about)
(2) the u-boot binary blob we were shipping doesn't correspond
to the sources we were shipping

and we fixed those in master by updating the blob and the
submodule to the most recent u-boot.

I guess the low-risk fix for the stable branch would be to
update the u-boot submodule to 79c884d7e4 as suggested in
https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg03174.html
which would bring the distributed sources into line with
the binary blob in stable, so no need to change the
blob we're distributing. I think it makes sense to do that
for stable.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] arm_gicv3_kvm: Fix compile warning

[Pranith Kumar]

CC'ing stable for 2.10.

On Tue, Aug 29, 2017 at 1:32 PM, Pranith Kumar  wrote:
> Fix the following warning:
>
> /home/pranith/qemu/hw/intc/arm_gicv3_kvm.c:296:17: warning: logical not is 
> only applied to the left hand side of this bitwise operator 
> [-Wlogical-not-parentheses]
> if (!c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) {
> ^ ~
> /home/pranith/qemu/hw/intc/arm_gicv3_kvm.c:296:17: note: add parentheses 
> after the '!' to evaluate the bitwise operator first
> if (!c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) {
> ^
> /home/pranith/qemu/hw/intc/arm_gicv3_kvm.c:296:17: note: add parentheses 
> around left hand side expression to silence this warning
> if (!c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) {
> ^
>
> Signed-off-by: Pranith Kumar 
> ---
>  hw/intc/arm_gicv3_kvm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> index 6051c77705..481fe5405a 100644
> --- a/hw/intc/arm_gicv3_kvm.c
> +++ b/hw/intc/arm_gicv3_kvm.c
> @@ -293,7 +293,7 @@ static void kvm_arm_gicv3_put(GICv3State *s)
>  kvm_gicr_access(s, GICR_PROPBASER + 4, ncpu, , true);
>
>  reg64 = c->gicr_pendbaser;
> -if (!c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) {
> +if (!(c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS)) {
>  /* Setting PTZ is advised if LPIs are disabled, to reduce
>   * GIC initialization time.
>   */
> --
> 2.11.0
>



-- 
Pranith

Re: [Qemu-devel] [PATCH 00/79] Patch Round-up for stable 2.9.1, freeze on 2017-09-04

[Michael Roth]

Quoting Thomas Huth (2017-08-28 21:18:20)
> On 29.08.2017 02:13, Michael Roth wrote:
> > Hi everyone,
> > 
> > The following new patches are queued for QEMU stable v2.9.1:
> > 
> >   
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_mdroth_qemu_commits_stable-2D2.9-2Dstaging=DwIDaQ=jf_iaSHvJObTbx-siA1ZOg=sThPI1c0u5x-3sg5Nw8wNqjg_5Z5xLzfPGC18E94zn8=Id5ItcTzhCqn35tC8JynLtLuRcfupmsTlJGwTYEDdIg=fqHIfooeKQPNEWX7AqyC93OMzAs-U-UwZ6Yu0trfn0Y=
> >  
> > 
> > The release is planned for 2017-09-07:
> > 
> >   
> > https://urldefense.proofpoint.com/v2/url?u=http-3A__wiki.qemu.org_Planning_2.9=DwIDaQ=jf_iaSHvJObTbx-siA1ZOg=sThPI1c0u5x-3sg5Nw8wNqjg_5Z5xLzfPGC18E94zn8=Id5ItcTzhCqn35tC8JynLtLuRcfupmsTlJGwTYEDdIg=RVxHUyHJDN1hk2AsMfiZmguXpEhz0pFHGijG75NIReY=
> >  
> > 
> > Please respond here or CC qemu-sta...@nongnu.org on any patches you
> > think should be included in the release.
> 
> I'd like to suggest the following patches:
> 
> 601b9a9008c5a612d76073bb - target-s390x: Mask the SIGP order_code ...
> b7da97eef74bf834be244de0 - monitor: Check whether TCG is enabled ...
> 17eb587aeb492fe68f8130b0 - slirp: tftp, copy sockaddr_size
> 99efaa2696caaf6182958e27 - hw/s390x/ipl: Fix crash with ...
> 36bed541ca886da735bef1e8 - fix qemu-system-unicore32 crashing ...
> b190f477e29c7cd03a8fee49 - qemu-system-tricore: segfault when ...
> 8ff9dd7ba24c7a788611 - hw/ppc/spapr_rtc: Mark the RTC device ...
> 1f98e55385d11da1dc0de644 - hw/ppc/spapr_iommu: Fix crash when ...
> 
> Not sure, but maybe the following patch should be included, too, since
> there were some bogus files in the old version of the U-Boot sources:
> 
> 73663d71ef2bab201475d58e - PPC: E500: Update u-boot to v2017.07

Do you have more background on any issues caused by these bogus files?
As it stands I think I would opt not to update unless there are specific
user-visible bugs we're trying to address which warrant the risk of any
regressions which might get pulled in in the process.

> 
>  Thomas
> 

Re: [Qemu-devel] [PATCH 0/2] hyperv: own protocol header instead of kernel's

[Roman Kagan]

On Thu, Jul 13, 2017 at 11:15:20PM +0300, Roman Kagan wrote:
> Supersede kernel's header for Hyper-V protocol definitions with our own.
> The reason is that, since this is a third-party protocol and not a
> kernel API, the kernel folks are not happy exposing it in the kernel
> uapi.
> 
> The patchset is done to allow gradual transition from the kernel's
> hyperv.h to the new one: the first patch introduces the new header that
> doesn't conflict with the old one, and the second makes sure the old one
> isn't imported from the kernel any longer, so that the maintainers can
> do the next round of kernel header import at their leisure and things
> will keep working.
> 
> Once (if) this lands in QEMU I'll post patches to KVM to move its
> hyperv.h out of uapi.
> 
> This is the first part split out of my former biggish "hyperv fixes and
> enhancements" series.  The only change is the underscore replaced with a
> dash in the header file name.
> 
> Roman Kagan (2):
>   hyperv: add header with protocol definitions
>   update-linux-headers: prepare for hyperv.h removal
> 
>  target/i386/cpu.h   |  10 +-
>  target/i386/hyperv-proto.h  | 257 
> 
>  target/i386/cpu.c   |   4 +-
>  target/i386/hyperv.c|   6 +-
>  target/i386/kvm.c   |  57 +
>  target/i386/machine.c   |  15 +--
>  scripts/update-linux-headers.sh |   4 +-
>  7 files changed, 304 insertions(+), 49 deletions(-)
>  create mode 100644 target/i386/hyperv-proto.h
> 
> -- 
> 2.13.0
> 
> 

Ping?

Roman.

Re: [Qemu-devel] [PATCH v2 3/3] docker: Add test-block

[Philippe Mathieu-Daudé]

On 08/31/2017 01:21 PM, Eric Blake wrote:

On 08/31/2017 11:18 AM, Daniel P. Berrange wrote:


'qcow' (v1) was also passing last time I checked, though it is
waay slow to run compared to qcow2, so I'd understand if you
were reluctant to turn that on due to speed


Even './check -qcow -g quick' might be nicer than nothing, but that is
also slooow


What about having a daily job running it on /master? Better than running 
it once before each release :)

Re: [Qemu-devel] [PATCH v2 3/3] docker: Add test-block

[Kashyap Chamarthy]

On Thu, Aug 31, 2017 at 11:06:55AM -0500, Eric Blake wrote:
> On 08/31/2017 06:22 AM, Fam Zheng wrote:
> > Signed-off-by: Fam Zheng 
> > ---
> >  tests/docker/test-block | 21 +
> >  1 file changed, 21 insertions(+)
> >  create mode 100755 tests/docker/test-block
> 
> 
> > +cd "$BUILD_DIR"
> > +
> > +build_qemu --target-list=x86_64-softmmu
> > +cd tests/qemu-iotests
> > +./check -raw
> > +./check -qcow2
> 
> Should we also run ./check -nbd?

FWIW, yes, if it's not too onerous.  Not least because many of the
NBD-using block layer features are important storage virtualization
features for management layers.

-- 
/kashyap

Re: [Qemu-devel] [PATCH v2 3/3] docker: Add test-block

[Eric Blake]

On 08/31/2017 11:18 AM, Daniel P. Berrange wrote:
> 
> 'qcow' (v1) was also passing last time I checked, though it is
> waay slow to run compared to qcow2, so I'd understand if you
> were reluctant to turn that on due to speed

Even './check -qcow -g quick' might be nicer than nothing, but that is
also slooow

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-2.11 v4 08/25] s390x: replace cpu_s390x_init() with cpu_generic_init()

[David Hildenbrand]

On 31.08.2017 16:04, Igor Mammedov wrote:
> On Thu, 31 Aug 2017 15:44:38 +0200
> David Hildenbrand  wrote:
> 
>> On 31.08.2017 15:19, Igor Mammedov wrote:
>>> cpu_s390x_init() is used only *-user targets indirectly
>>> via cpu_init() macro and has a hack to assign ids to created
>>> cpus (I'm not sure if 'id' really matters to *-user emulation).  
>>
>> It only has one cpu, right? With the below mentioned patch, the default
>> "id"/ (the cpu_addr) will be 0. So you can create one CPU without any
>> hacks. Or can it create more than 1?
> As I understand it creates only one cpu,
> but then there is fork related cpu_copy() which creates a cpu
> and the rewrites new CPUArchState with old cpu data
> 
> CPUState *new_cpu = cpu_init(cpu_model);  
>
> CPUArchState *new_env = new_cpu->env_ptr; 
>
> ...
> memcpy(new_env, env, sizeof(CPUArchState));
> 
> so I couldn't decide if id is of importance for *-user target
> (well, I'm not really interested in *-user), and opted for
> keeping current behavior (i.e. not breaking) in this patch
> just without cpu_s390x_init().
> 
> So if 'id' isn't really needed for  *-user target, hunk 2 could be
> dropped but it ought to be another patch on top as this change isn't
> related to the topic of this patch/series.
>  

Indeed, it seems to create a new cpu for every fork. So this is necessary.

-- 

Thanks,

David

Re: [Qemu-devel] [PATCH 2/2] .dir-locals.el: add json schema to auto-mode

[Eric Blake]

On 08/31/2017 08:14 AM, Marc-André Lureau wrote:
> The json schema is more friendly to python-mode since it doesn't
> follow strictly json (comments to start with).

I found that hard to read, and missing a key word of 'emacs'.  Maybe
reword it as:

Under emacs, our QAPI files (.json suffix) are more friendly to edit
under python-mode than the per-suffix default of json-mode (in part
because of our use of comments).

> 
> All schema files add file variables to set the python mode, but most
> tests didn't. This will cover all json files now.
> 
> Signed-off-by: Marc-André Lureau 
> ---

> +++ b/.dir-locals.el
> @@ -2,6 +2,8 @@
>  (indent-tabs-mode . nil)))
>   (nil . ((eval . (setq auto-mode-alist (append
>  '(("Makefile.*" . 
> makefile-gmake-mode)
> -  ("\\.mak\\'" . 
> makefile-gmake-mode))
> +  ("\\.mak\\'" . makefile-gmake-mode)
> +  ;; json schema is not pure json
> +  ("\\.json\\'" . python-mode))
>  auto-mode-alist
>))

Not sure if the comment could be worded better, maybe:

;; Our .json files are QAPI, and allow more than pure json

Again, the idea makes sense to me, but you may want to get feedback from
other power emacs users.

[Also, don't forget the 0/2 cover letter if you send v2]

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 5/9] IDE: replace DEBUG_AIO with trace events

[Laszlo Ersek]

On 08/31/17 01:25, John Snow wrote:
> CCing Laszlo Ersek literally just for laughs, as he is the most
> entertaining language lawyer I know of.
> 
> Laszlo, please feel free to ignore this if you don't care :P
> 
> On 08/29/2017 11:14 PM, Philippe Mathieu-Daudé wrote:
>> Hi John,
>>
>> On 08/29/2017 05:49 PM, John Snow wrote:
>>> Signed-off-by: John Snow 
>>> ---
>>>   hw/ide/atapi.c|  5 +
>>>   hw/ide/core.c | 17 ++---
>>>   hw/ide/trace-events   |  3 +++
>>>   include/hw/ide/internal.h |  6 --
>>>   4 files changed, 18 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
>>> index 37fa699..b8fc51e 100644
>>> --- a/hw/ide/atapi.c
>>> +++ b/hw/ide/atapi.c
>>> @@ -416,10 +416,7 @@ static void ide_atapi_cmd_read_dma_cb(void
>>> *opaque, int ret)
>>>   s->io_buffer_size = n * 2048;
>>>   data_offset = 0;
>>>   }
>>> -#ifdef DEBUG_AIO
>>> -printf("aio_read_cd: lba=%u n=%d\n", s->lba, n);
>>> -#endif
>>> -
>>> +trace_ide_atapi_cmd_read_dma_cb_aio(s, s->lba, n);
>>>   s->bus->dma->iov.iov_base = (void *)(s->io_buffer + data_offset);
>>>   s->bus->dma->iov.iov_len = n * ATAPI_SECTOR_SIZE;
>>>   qemu_iovec_init_external(>bus->dma->qiov, >bus->dma->iov, 1);
>>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>>> index 82a19b1..a1c90e9 100644
>>> --- a/hw/ide/core.c
>>> +++ b/hw/ide/core.c
>>> @@ -58,6 +58,13 @@ static const int smart_attributes[][12] = {
>>>   { 190,  0x03, 0x00, 0x45, 0x45, 0x1f, 0x00, 0x1f, 0x1f, 0x00,
>>> 0x00, 0x32},
>>>   };
>>>   +const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT] = {
>>> +[IDE_DMA_READ] = "DMA READ",
>>> +[IDE_DMA_WRITE] = "DMA WRITE",
>>> +[IDE_DMA_TRIM] = "DMA TRIM",
>>> +[IDE_DMA_ATAPI] = "DMA ATAPI"
>>> +};
>>> +
>>>   static void ide_dummy_transfer_stop(IDEState *s);
>>> static void padstr(char *str, const char *src, int len)
>>> @@ -860,10 +867,8 @@ static void ide_dma_cb(void *opaque, int ret)
>>>   goto eot;
>>>   }
>>>   -#ifdef DEBUG_AIO
>>> -printf("ide_dma_cb: sector_num=%" PRId64 " n=%d, cmd_cmd=%d\n",
>>> -   sector_num, n, s->dma_cmd);
>>> -#endif
>>> +trace_ide_dma_cb(s, sector_num, n,
>>> + IDE_DMA_CMD_lookup[s->dma_cmd]);
>>> if ((s->dma_cmd == IDE_DMA_READ || s->dma_cmd ==
>>> IDE_DMA_WRITE) &&
>>>   !ide_sect_range_ok(s, sector_num, n)) {
>>> @@ -2391,9 +2396,7 @@ void ide_bus_reset(IDEBus *bus)
>>> /* pending async DMA */
>>>   if (bus->dma->aiocb) {
>>> -#ifdef DEBUG_AIO
>>> -printf("aio_cancel\n");
>>> -#endif
>>> +trace_ide_bus_reset_aio();
>>>   blk_aio_cancel(bus->dma->aiocb);
>>>   bus->dma->aiocb = NULL;
>>>   }
>>> diff --git a/hw/ide/trace-events b/hw/ide/trace-events
>>> index 8c79a6c..cc8949c 100644
>>> --- a/hw/ide/trace-events
>>> +++ b/hw/ide/trace-events
>>> @@ -18,6 +18,8 @@ ide_cancel_dma_sync_remaining(void) "draining all
>>> remaining requests"
>>>   ide_sector_read(int64_t sector_num, int nsectors) "sector=%"PRId64"
>>> nsectors=%d"
>>>   ide_sector_write(int64_t sector_num, int nsectors) "sector=%"PRId64"
>>> nsectors=%d"
>>>   ide_reset(void *s) "IDEstate %p"
>>> +ide_bus_reset_aio(void) "aio_cancel"
>>> +ide_dma_cb(void *s, int64_t sector_num, int n, const char *dma)
>>> "IDEState %p; sector_num=%"PRId64" n=%d cmd=%s"
>>> # BMDMA HBAs:
>>>   @@ -51,5 +53,6 @@ ide_atapi_cmd_reply_end_new(void *s, int status)
>>> "IDEState: %p; new transfer sta
>>>   ide_atapi_cmd_check_status(void *s) "IDEState: %p"
>>>   ide_atapi_cmd_read(void *s, const char *method, int lba, int
>>> nb_sectors) "IDEState: %p; read %s: LBA=%d nb_sectors=%d"
>>>   ide_atapi_cmd(void *s, uint8_t cmd) "IDEState: %p; cmd: 0x%02x"
>>> +ide_atapi_cmd_read_dma_cb_aio(void *s, int lba, int n) "IDEState: %p;
>>> aio read: lba=%d n=%d"
>>>   # Warning: Verbose
>>>   ide_atapi_cmd_packet(void *s, uint16_t limit, const char *packet)
>>> "IDEState: %p; limit=0x%x packet: %s"
>>> diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
>>> index 74efe8a..db9fde0 100644
>>> --- a/include/hw/ide/internal.h
>>> +++ b/include/hw/ide/internal.h
>>> @@ -14,7 +14,6 @@
>>>   #include "block/scsi.h"
>>> /* debug IDE devices */
>>> -//#define DEBUG_AIO
>>>   #define USE_DMA_CDROM
>>> typedef struct IDEBus IDEBus;
>>> @@ -333,12 +332,15 @@ struct unreported_events {
>>>   };
>>> enum ide_dma_cmd {
>>> -IDE_DMA_READ,
>>> +IDE_DMA_READ = 0,
>>>   IDE_DMA_WRITE,
>>>   IDE_DMA_TRIM,
>>>   IDE_DMA_ATAPI,
>>> +IDE_DMA__COUNT
>>>   };
>>>   +extern const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT];
>>
>> I recommend you to avoid this declaring extern const array with size, I
>> remember some compilers (old GCC?) ignoring array size in extern. Eric
>> will correct me!
>>
>> It is much safer to use a getter:
>>
> 
> Well, whether or not the compiler ignores it, you're right that it's
> safer 

Re: [Qemu-devel] [PATCH v2 3/3] docker: Add test-block

[Daniel P. Berrange]

On Thu, Aug 31, 2017 at 07:22:10PM +0800, Fam Zheng wrote:
> Signed-off-by: Fam Zheng 
> ---
>  tests/docker/test-block | 21 +
>  1 file changed, 21 insertions(+)
>  create mode 100755 tests/docker/test-block
> 
> diff --git a/tests/docker/test-block b/tests/docker/test-block
> new file mode 100755
> index 00..20ef70538f
> --- /dev/null
> +++ b/tests/docker/test-block
> @@ -0,0 +1,21 @@
> +#!/bin/bash -e
> +#
> +# Run block test cases
> +#
> +# Copyright (c) 2017 Red Hat Inc.
> +#
> +# Authors:
> +#  Fam Zheng 
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2
> +# or (at your option) any later version. See the COPYING file in
> +# the top-level directory.
> +
> +. common.rc
> +
> +cd "$BUILD_DIR"
> +
> +build_qemu --target-list=x86_64-softmmu
> +cd tests/qemu-iotests
> +./check -raw
> +./check -qcow2

Could you add  'check -luks' - it should be passing already with git
master.  Ideally it wants password-less sudo enabled, but will skip
the appropriate test if that's not there.

'qcow' (v1) was also passing last time I checked, though it is
waay slow to run compared to qcow2, so I'd understand if you
were reluctant to turn that on due to speed

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 1/2] .dir-locals.el: add makefiles to auto-mode

[Eric Blake]

On 08/31/2017 08:14 AM, Marc-André Lureau wrote:
> QEMU uses non-conventional makefile filenames. Ease the life of emacs
> developpers a bit by appending to the auto-mode-alist.

s/emacs developpers/developers using emacs/ (typo, and avoids an
ambiguity on whether you are describing people that develop emacs rather
than qemu)

> 
> Signed-off-by: Marc-André Lureau 
> ---

The idea itself makes sense to me; although I'm not enough of a Lisp
guru to state if your changes are the ideal way to do it.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v1 06/11] target/s390x: cleanup cpu number/address handling

[Cornelia Huck]

On Thu, 31 Aug 2017 18:03:52 +0200
Igor Mammedov  wrote:

> On Thu, 31 Aug 2017 16:35:13 +0200
> Cornelia Huck  wrote:
> 
> > On Wed, 30 Aug 2017 19:05:56 +0200
> > David Hildenbrand  wrote:
> >   
> > > Some time ago we discussed that using "id" as property name is not the
> > > right thing to do, as it is a reserved property for other devices.
> > > 
> > > Switch to the term "addr" instead, which matches the definition in the
> > > PoP called "CPU address". There is no such thing as cpu number, so
> > > rename env.cpu_num to env.cpu_addr.
> > > 
> > > We can get rid of cpu->id now. Keep cpu->index and env->cpu_addr in sync.
> > > cpu->index was already implicitly used by e.g. cpu_exists(), so keeping
> > > both in sync seems to be the right thing to do.
> > > 
> > > cpu->index will now no longer automatically get set via
> > > cpu_exec_realizefn(). For now, we were lucky that both implicitly stayed
> > > in sync.
> > > 
> > > Our new cpu property "addr" can be a static property. Range checks can
> > > be avoided by using the correct type and the "setting after realized"
> > > check is done implicitly.
> > > 
> > > AFAIK, s390x only supports cpu_add and not device_add for cpus. So we
> > > should be able to safely rename that property (no the "id" property
> > > could properly be used for device_add, which needs an artificial id for
> > > identification purposes).
> this patch seems to somewhat conflicting with
>  https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg06505.html
> that were supposed to go via machine tree and which I've respinned today
> to fix conflicts due just merged pull req.
> 
> Cornelia,
> 
> Could you put/merge it via s390x tree so that David and I work won't clash 
> again?

I can do that, sure. I can put your patch on top of David's.

Re: [Qemu-devel] [PULL 00/29] Code cleanup patches

[Peter Maydell]

On 31 August 2017 at 11:34, Marc-André Lureau
 wrote:
> The following changes since commit 1201d308519f1e915866d7583d5136d03cc1d384:
>
>   slirp: fix clearing ifq_so from pending packets (2017-08-30 23:14:34 +0100)
>
> are available in the git repository at:
>
>   https://github.com/elmarco/qemu.git tags/tidy-pull-request
>
> for you to fetch changes up to e4d67e4f2e06128bc7f07263afe9acc2e2242fdc:
>
>   eepro100: replace g_malloc()+memcpy() with g_memdup() (2017-08-31 12:29:07 
> +0200)
>
> 
>

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v1 08/11] s390x: allow only 1 CPU with TCG

[Cornelia Huck]

On Thu, 31 Aug 2017 17:33:13 +0200
David Hildenbrand  wrote:

> On 31.08.2017 17:07, Cornelia Huck wrote:

> > If we know that this can't work, it makes sense to stop immediately, no?  
> 
> Hmm, like that than?
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 18ed0c57e3..dae848fa5f 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -23,6 +23,7 @@
>  #include "hw/s390x/css.h"
>  #include "virtio-ccw.h"
>  #include "qemu/config-file.h"
> +#include "qemu/error-report.h"
>  #include "s390-pci-bus.h"
>  #include "hw/s390x/storage-keys.h"
>  #include "hw/s390x/storage-attributes.h"
> @@ -56,6 +57,12 @@ static void s390_init_cpus(MachineState *machine)
>  if (machine->cpu_model == NULL) {
>  machine->cpu_model = s390_default_cpu_model_name();
>  }
> +if (tcg_enabled() && max_cpus > S390_TCG_MAX_CPUS) {
> +error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
> + "supported by TCG (%d) on s390x", max_cpus,
> + S390_TCG_MAX_CPUS);
> +exit(1);
> +}
> 
>  ms->cpus = g_new0(S390CPU *, max_cpus);
> 
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 7ed9103b33..4e1de2102d 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -210,6 +210,8 @@ static inline S390CPU
> *s390_env_get_cpu(CPUS390XState *env)
> 
>  #define ENV_OFFSET offsetof(S390CPU, env)
> 
> +#define S390_TCG_MAX_CPUS 1
> +
>  #ifndef CONFIG_USER_ONLY
>  extern const struct VMStateDescription vmstate_s390_cpu;
>  #endif
> 
> 

Looks good!

Re: [Qemu-devel] [PATCH v2 3/3] docker: Add test-block

[Eric Blake]

On 08/31/2017 06:22 AM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng 
> ---
>  tests/docker/test-block | 21 +
>  1 file changed, 21 insertions(+)
>  create mode 100755 tests/docker/test-block


> +cd "$BUILD_DIR"
> +
> +build_qemu --target-list=x86_64-softmmu
> +cd tests/qemu-iotests
> +./check -raw
> +./check -qcow2

Should we also run ./check -nbd?

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 3/5] qapi: Use OrderedDict from standard library if available

[Eric Blake]

On 08/31/2017 09:24 AM, Daniel P. Berrange wrote:
> The OrderedDict class appeared in the 'collections' module
> from python 2.7 onwards, so use that in preference to our
> local backport if available.

Since we're now using argparse.py as a third-party import (commit
47e1cb1f) also for the sake of 2.6, can we treat ordereddict.py as the
same sort of third-party import?

> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  scripts/qapi.py | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index d89af7d6c6..05cb1ee38c 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -18,7 +18,10 @@ import os
>  import re
>  import string
>  import sys
> -from ordereddict import OrderedDict
> +try:
> +from collections import OrderedDict
> +except:
> +from ordereddict import OrderedDict
>  
>  builtin_types = {
>  'null': 'QTYPE_QNULL',
> 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v1 06/11] target/s390x: cleanup cpu number/address handling

[Igor Mammedov]

On Thu, 31 Aug 2017 16:35:13 +0200
Cornelia Huck  wrote:

> On Wed, 30 Aug 2017 19:05:56 +0200
> David Hildenbrand  wrote:
> 
> > Some time ago we discussed that using "id" as property name is not the
> > right thing to do, as it is a reserved property for other devices.
> > 
> > Switch to the term "addr" instead, which matches the definition in the
> > PoP called "CPU address". There is no such thing as cpu number, so
> > rename env.cpu_num to env.cpu_addr.
> > 
> > We can get rid of cpu->id now. Keep cpu->index and env->cpu_addr in sync.
> > cpu->index was already implicitly used by e.g. cpu_exists(), so keeping
> > both in sync seems to be the right thing to do.
> > 
> > cpu->index will now no longer automatically get set via
> > cpu_exec_realizefn(). For now, we were lucky that both implicitly stayed
> > in sync.
> > 
> > Our new cpu property "addr" can be a static property. Range checks can
> > be avoided by using the correct type and the "setting after realized"
> > check is done implicitly.
> > 
> > AFAIK, s390x only supports cpu_add and not device_add for cpus. So we
> > should be able to safely rename that property (no the "id" property
> > could properly be used for device_add, which needs an artificial id for
> > identification purposes).  
this patch seems to somewhat conflicting with
 https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg06505.html
that were supposed to go via machine tree and which I've respinned today
to fix conflicts due just merged pull req.

Cornelia,

Could you put/merge it via s390x tree so that David and I work won't clash 
again?

Re: [Qemu-devel] [PATCH v5 04/12] tests: Add vm test lib

[Philippe Mathieu-Daudé]

Hi Fam,

On 08/31/2017 03:42 AM, Fam Zheng wrote:

This is the common code to implement a "VM test" to

   1) Download and initialize a pre-defined VM that has necessary
   dependencies to build QEMU and SSH access.


Looking at this project:

https://www.packer.io/docs/builders/qemu.html

Maybe we can have another CI that prepare/upload those images 
(reproducible, easy upgrade version...), and document the build process 
in the JSON instead of the wiki (so the doc is up to date).




   2) Archive $SRC_PATH to a .tar file.

   3) Boot the VM, and pass the source tar file to the guest.

   4) SSH into the VM, untar the source tarball, build from the source.

Signed-off-by: Fam Zheng 
---
  tests/vm/basevm.py | 276 +
  1 file changed, 276 insertions(+)
  create mode 100755 tests/vm/basevm.py

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
new file mode 100755
index 00..9db91d61fa
--- /dev/null
+++ b/tests/vm/basevm.py
@@ -0,0 +1,276 @@
+#!/usr/bin/env python
+#
+# VM testing base class
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import logging
+import time
+import datetime
+sys.path.append(os.path.join(os.path.dirname(__file__), "..", "..", "scripts"))
+from qemu import QEMUMachine
+import subprocess
+import hashlib
+import optparse
+import atexit
+import tempfile
+import shutil
+import multiprocessing
+import traceback
+
+SSH_KEY = """\
+-BEGIN RSA PRIVATE KEY-
+MIIEowIBAAKCAQEAopAuOlmLV6LVHdFBj8/eeOwI9CqguIJPp7eAQSZvOiB4Ag/R
+coEhl/RBbrV5Yc/SmSD4PTpJO/iM10RwliNjDb4a3I8q3sykRJu9c9PI/YsH8WN9
++NH2NjKPtJIcKTu287IM5JYxyB6nDoOzILbTyJ1TDR/xH6qYEfBAyiblggdjcvhA
+RTf93QIn39F/xLypXvT1K2O9BJEsnJ8lEUvB2UXhKo/JTfSeZF8wPBeowaP9EONk
+7b+nuJOWHGg68Ji6wVi62tjwl2Szch6lxIhZBpnV7QNRKMfYHP6eIyF4pusazzZq
+Telsq6xI2ghecWLzb/MF5A+rklsGx2FNuJSAJwIDAQABAoIBAHHi4o/8VZNivz0x
+cWXn8erzKV6tUoWQvW85Lj/2RiwJvSlsnYZDkx5af1CpEE2HA/pFT8PNRqsd+MWC
+7AEy710cVsM4BYerBFYQaYxwzblaoojo88LSjVPw3h5Z0iLM8+IMVd36nwuc9dpE
+R8TecMZ1+U4Tl6BgqkK+9xToZRdPKdjS8L5MoFhGN+xY0vRbbJbGaV9Q0IHxLBkB
+rEBV7T1mUynneCHRUQlJQEwJmKpT8MH3IjsUXlG5YvnuuvcQJSNTaW2iDLxuOKp8
+cxW8+qL88zpb1D5dppoIu6rlrugN0azSq70ruFJQPc/A8GQrDKoGgRQiagxNY3u+
+vHZzXlECgYEA0dKO3gfkSxsDBb94sQwskMScqLhcKhztEa8kPxTx6Yqh+x8/scx3
+XhJyOt669P8U1v8a/2Al+s81oZzzfQSzO1Q7gEwSrgBcRMSIoRBUw9uYcy02ngb/
+j/ng3DGivfJztjjiSJwb46FHkJ2JR8mF2UisC6UMXk3NgFY/3vWQx78CgYEAxlcG
+T3hfSWSmTgKRczMJuHQOX9ULfTBIqwP5VqkkkiavzigGRirzb5lgnmuTSPTpF0LB
+XVPjR2M4q+7gzP0Dca3pocrvLEoxjwIKnCbYKnyyvnUoE9qHv4Kr+vDbgWpa2LXG
+JbLmE7tgTCIp20jOPPT4xuDvlbzQZBJ5qCQSoZkCgYEAgrotSSihlCnAOFSTXbu4
+CHp3IKe8xIBBNENq0eK61kcJpOxTQvOha3sSsJsU4JAM6+cFaxb8kseHIqonCj1j
+bhOM/uJmwQJ4el/4wGDsbxriYOBKpyq1D38gGhDS1IW6kk3erl6VAb36WJ/OaGum
+eTpN9vNeQWM4Jj2WjdNx4QECgYAwTdd6mU1TmZCrJRL5ZG+0nYc2rbMrnQvFoqUi
+BvWiJovggHzur90zy73tNzPaq9Ls2FQxf5G1vCN8NCRJqEEjeYCR59OSDMu/EXc2
+CnvQ9SevHOdS1oEDEjcCWZCMFzPi3XpRih1gptzQDe31uuiHjf3cqcGPzTlPdfRt
+D8P92QKBgC4UaBvIRwREVJsdZzpIzm224Bpe8LOmA7DeTnjlT0b3lkGiBJ36/Q0p
+VhYh/6cjX4/iuIs7gJbGon7B+YPB8scmOi3fj0+nkJAONue1mMfBNkba6qQTc6Y2
+5mEKw2/O7/JpND7ucU3OK9plcw/qnrWDgHxl0Iz95+OzUIIagxne
+-END RSA PRIVATE KEY-
+"""
+SSH_PUB_KEY = """\
+ssh-rsa 
B3NzaC1yc2EDAQABAAABAQCikC46WYtXotUd0UGPz9547Aj0KqC4gk+nt4BBJm86IHgCD9FygSGX9EFutXlhz9KZIPg9Okk7+IzXRHCWI2MNvhrcjyrezKREm71z08j9iwfxY3340fY2Mo+0khwpO7bzsgzkljHIHqcOg7MgttPInVMNH/EfqpgR8EDKJuWCB2Ny+EBFN/3dAiff0X/EvKle9PUrY70EkSycnyURS8HZReEqj8lN9J5kXzA8F6jBo/0Q42Ttv6e4k5YcaDrwmLrBWLra2PCXZLNyHqXEiFkGmdXtA1Eox9gc/p4jIXim6xrPNmpN6WyrrEjaCF5xYvNv8wXkD6uSWwbHYU24lIAn
 qemu-vm-key
+"""
+
+class BaseVM(object):
+GUEST_USER = "qemu"
+GUEST_PASS = "qemupass"
+ROOT_PASS = "qemupass"
+
+# The script to run in the guest that builds QEMU
+BUILD_SCRIPT = ""
+# The guest name, to be overridden by subclasses
+name = "#base"
+def __init__(self, debug=False, vcpus=None):
+self._guest = None
+self._tmpdir = tempfile.mkdtemp(prefix="vm-test-", suffix=".tmp", 
dir=".")
+atexit.register(shutil.rmtree, self._tmpdir)
+
+self._ssh_key_file = os.path.join(self._tmpdir, "id_rsa")
+open(self._ssh_key_file, "w").write(SSH_KEY)
+subprocess.check_call(["chmod", "600", self._ssh_key_file])
+
+self._ssh_pub_key_file = os.path.join(self._tmpdir, "id_rsa.pub")
+open(self._ssh_pub_key_file, "w").write(SSH_PUB_KEY)
+
+self.debug = debug
+self._stderr = sys.stderr
+self._devnull = open("/dev/null", "w")
+if self.debug:
+self._stdout = sys.stdout
+else:
+self._stdout = self._devnull
+self._args = [ \
+"-nodefaults", "-m", "2G",
+"-cpu", "host",
+"-netdev", "user,id=vnet,hostfwd=:0.0.0.0:0-:22",
+"-device", 

Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests attempt to leave intermediate files

[Jeff Cody]

On Thu, Aug 31, 2017 at 04:39:49PM +0100, Stefan Hajnoczi wrote:
> On Wed, Aug 30, 2017 at 06:40:29PM -0400, John Snow wrote:
> > 
> > 
> > On 08/30/2017 06:35 PM, Eric Blake wrote:
> > > On 08/30/2017 05:28 PM, John Snow wrote:
> > > 
> > >> I'm a little iffy on this patch; I know that ./check can take care of
> > >> our temp files for us now, but because each python test is itself a
> > >> little mini-harness, I'm a little leery of moving the teardown to setup
> > >> and trying to pre-clean the confetti before the test begins.
> > >>
> > >> What's the benefit? We still have to clean up these files per-test, but
> > >> now it's slightly more error-prone and in a weird place.
> > >>
> > >> If we want to try to preserve the most-recent-failure-files, perhaps we
> > >> can define a setting in the python test-runner that allows us to
> > >> globally skip file cleanup.
> > > 
> > > On the other hand, since each test is a mini-harness, globally skipping
> > > cleanup will make a two-part test fail on the second because of garbage
> > > left behind by the first.
> > > 
> > 
> > subtext was to have per-subtest files.
> > 
> > > Patch 5 adds a comment with another possible solution: teach the python
> > > mini-harness to either clean all files in the directory, or to relocate
> > > the directory according to test name, so that each mini-test starts with
> > > a fresh location, and cleanup is then handled by the harness rather than
> > > spaghetti pre-cleanup.  But any solution is better than our current
> > > situation of nothing, so that's why I'm still okay with this patch as-is
> > > as offering more (even if not perfect) than before.
> > > 
> > 
> > I guess where I am unsure is really if this is better than what we
> > currently do, which is to (try) to clean up after each test as best as
> > we can. I don't see it as too different from trying to clean up before
> > each test.
> > 
> > It does give us the ability to leave behind a little detritus after a
> > failed run, but it's so imperfect that I wonder if it's worth shifting
> > this code around to change not much.
> 
> An alternative is to define iotests.QMPTestCase.setUp() so it clears out
> iotests.test_dir.  Unfortunately this still requires touching up all
> setUp() methods so that they call super(TheClass, self).setUp().
> 
> At least there would be no need to delete specific files by name (e.g.
> blind_remove(my_img)).
> 

One reason to only remove specific files used in the test, is that it
increases the chance that intermediate files will be left behind in case of
test failure of a different test case.

I think the real long-term solution is to run each unittest test case in its
own subdirectory, so that no intermediate file removal is necessary, and
each test case is self-contained.

-Jeff

Re: [Qemu-devel] RFC: changing ROM bundling in tar dists for releases

[Philippe Mathieu-Daudé]

On 08/31/2017 09:29 AM, Daniel P. Berrange wrote:
[...]> There are the following options I see


   1. Keep existing dist, and add a new minimal one

qemu-X.Y.Z.tar.bz2 - qemu source + bundled ROMS + libs
qemu-minimal-X.Y.Z.tar.bz2 - qemu source only

  Least impact for current non-distro users, distros just switch.

   2. Change existing dist, and add a new one with everything

qemu-X.Y.Z.tar.bz2 - qemu source only
qemu-full-X.Y.Z.tar.bz2 - qemu source + bundled ROMS + libs

  Non-distro users need to download a different dist from what they
  have known previously, but otherwise unchanged build process.

   3. Change existing dist, and add a new one with bundled bits

qemu-X.Y.Z.tar.bz2 - qemu source only
qemu-addons-X.Y.Z.tar.bz2 - bundled ROMS + libs

  Non-distro users need to manually download & install an
  extra piece compared to now.


Debian style:

linux-kbuild-4.9 - Kbuild infrastructure for Linux 4.9
linux-kbuild-4.12 - Kbuild infrastructure for Linux 4.12
linux-source - Linux kernel source (meta-package)
linux-source-4.12 - Linux kernel source for version 4.12 with Debian patches
firmware-linux - Binary firmware for various drivers in the Linux kernel 
(meta-package)
firmware-linux-nonfree - Binary firmware for various drivers in the 
Linux kernel (meta-package)
firmware-misc-nonfree - Binary firmware for various drivers in the Linux 
kernel

firmware-zd1211 - binary firmware for the zd1211rw wireless driver
firmware-bnx2 - Binary firmware for Broadcom NetXtremeII
...
qemu-efi - UEFI firmware for 64-bit ARM virtual machines
grub-firmware-qemu - GRUB firmware image for QEMU
ipxe-qemu - PXE boot firmware - ROM images for qemu
ovmf - UEFI firmware for 64-bit x86 virtual machines
vgabios - VGA BIOS software for the Bochs and Qemu emulated VGA card
seabios - Legacy BIOS implementation
sgabios - bios option rom to provide legacy serial console for x86

$ apt-cache show qemu-system-x86
Package: qemu-system-x86
Provides: qemu-system-i386, qemu-system-x86-64
Suggests: samba, vde2, qemu-block-extra, kmod, sgabios, ovmf

$ apt-cache show qemu-system-arm
Recommends: qemu-utils, ipxe-qemu, qemu-efi

This seems to me the cleaner one, the distrib choose to provide 
dependencies in the package description, a package to build, and keep 
exclusive licenses in different packages.




   4. Change existing dist, never distribute bundled ROMs + libs at all

qemu-X.Y.Z.tar.bz2 - qemu source only

  Only here for completeness, not a serious suggestion


Linux style :)



   5. Change nothing

  Continue ignoring the problem I aim to solve.


My patch did option 2, but I really open to any of 1/2/3 without a
strong preference, as long as we can get some agreement on one. I
guess option 1 might be easiest to get agreement on given its minimal
impact.


It would be easier to have people building from sources to answer but 
they usually don't read this list, you might want to ask on qemu-stable.


Regards,

Phil.

Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests attempt to leave intermediate files

[Stefan Hajnoczi]

On Wed, Aug 30, 2017 at 06:40:29PM -0400, John Snow wrote:
> 
> 
> On 08/30/2017 06:35 PM, Eric Blake wrote:
> > On 08/30/2017 05:28 PM, John Snow wrote:
> > 
> >> I'm a little iffy on this patch; I know that ./check can take care of
> >> our temp files for us now, but because each python test is itself a
> >> little mini-harness, I'm a little leery of moving the teardown to setup
> >> and trying to pre-clean the confetti before the test begins.
> >>
> >> What's the benefit? We still have to clean up these files per-test, but
> >> now it's slightly more error-prone and in a weird place.
> >>
> >> If we want to try to preserve the most-recent-failure-files, perhaps we
> >> can define a setting in the python test-runner that allows us to
> >> globally skip file cleanup.
> > 
> > On the other hand, since each test is a mini-harness, globally skipping
> > cleanup will make a two-part test fail on the second because of garbage
> > left behind by the first.
> > 
> 
> subtext was to have per-subtest files.
> 
> > Patch 5 adds a comment with another possible solution: teach the python
> > mini-harness to either clean all files in the directory, or to relocate
> > the directory according to test name, so that each mini-test starts with
> > a fresh location, and cleanup is then handled by the harness rather than
> > spaghetti pre-cleanup.  But any solution is better than our current
> > situation of nothing, so that's why I'm still okay with this patch as-is
> > as offering more (even if not perfect) than before.
> > 
> 
> I guess where I am unsure is really if this is better than what we
> currently do, which is to (try) to clean up after each test as best as
> we can. I don't see it as too different from trying to clean up before
> each test.
> 
> It does give us the ability to leave behind a little detritus after a
> failed run, but it's so imperfect that I wonder if it's worth shifting
> this code around to change not much.

An alternative is to define iotests.QMPTestCase.setUp() so it clears out
iotests.test_dir.  Unfortunately this still requires touching up all
setUp() methods so that they call super(TheClass, self).setUp().

At least there would be no need to delete specific files by name (e.g.
blind_remove(my_img)).

Stefan

Re: [Qemu-devel] [PATCH v2] tests: Enable the drive_del test also on s390x

[David Hildenbrand]

>  static void test_drive_del_device_del(void)
>  {
> +char *args;
> +
>  /* Start with a drive used by a device that unplugs instantaneously */
> -qtest_start("-drive if=none,id=drive0,file=null-co://,format=raw"
> -" -device virtio-scsi-pci"
> -" -device scsi-hd,drive=drive0,id=dev0");
> +args = g_strdup_printf("-drive 
> if=none,id=drive0,file=null-co://,format=raw"
> +   " -device virtio-scsi-%s"
> +   " -device scsi-hd,drive=drive0,id=dev0",

Would look better with the spaces at the end of the previous line (so
all "-device" are aligned), but just my taste.

> +   qvirtio_get_dev_type());
> +qtest_start(args);
>  
>  /*
>   * Delete the drive, and then the device
> @@ -104,6 +109,7 @@ static void test_drive_del_device_del(void)
>  device_del();
>  
>  qtest_end();
> +g_free(args);
>  }
>  
>  int main(int argc, char **argv)
> @@ -114,9 +120,10 @@ int main(int argc, char **argv)
>  
>  qtest_add_func("/drive_del/without-dev", test_drive_without_dev);
>  
> -/* TODO I guess any arch with PCI would do */
> +/* TODO I guess any arch with a hot-pluggable virtio bus would do */
>  if (!strcmp(arch, "i386") || !strcmp(arch, "x86_64") ||
> -!strcmp(arch, "ppc") || !strcmp(arch, "ppc64")) {
> +!strcmp(arch, "ppc") || !strcmp(arch, "ppc64") ||
> +!strcmp(arch, "s390x")) {
>  qtest_add_func("/drive_del/after_failed_device_add",
> test_after_failed_device_add);
>  qtest_add_func("/blockdev/drive_del_device_del",
> diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
> index 9880a69..0879a62 100644
> --- a/tests/libqos/virtio.c
> +++ b/tests/libqos/virtio.c
> @@ -339,3 +339,20 @@ void qvirtqueue_set_used_event(QVirtQueue *vq, uint16_t 
> idx)
>  /* vq->avail->used_event */
>  writew(vq->avail + 4 + (2 * vq->size), idx);
>  }
> +
> +/*
> + * qvirtio_get_dev_type:
> + * Returns: the preferred virtio bus/device type for the current 
> architecture.
> + */
> +const char *qvirtio_get_dev_type(void)
> +{
> +const char *arch = qtest_get_arch();
> +
> +if (g_str_equal(arch, "arm") || g_str_equal(arch, "aarch64")) {
> +return "device";  /* for virtio-mmio */
> +} else if (g_str_equal(arch, "s390x")) {
> +return "ccw";
> +} else {
> +return "pci";
> +}

You could drop the else case and do it unconditionally.

> +}
> diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
> index 8fbcd18..0a04740 100644
> --- a/tests/libqos/virtio.h
> +++ b/tests/libqos/virtio.h
> @@ -143,4 +143,7 @@ void qvirtqueue_kick(QVirtioDevice *d, QVirtQueue *vq, 
> uint32_t free_head);
>  bool qvirtqueue_get_buf(QVirtQueue *vq, uint32_t *desc_idx);
>  
>  void qvirtqueue_set_used_event(QVirtQueue *vq, uint16_t idx);
> +
> +const char *qvirtio_get_dev_type(void);
> +
>  #endif
> 


Looks good to me!

Reviewed-by: David Hildenbrand 

-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v1 08/11] s390x: allow only 1 CPU with TCG

[David Hildenbrand]

On 31.08.2017 17:07, Cornelia Huck wrote:
> On Thu, 31 Aug 2017 17:03:21 +0200
> David Hildenbrand  wrote:
> 
>> On 31.08.2017 16:41, Cornelia Huck wrote:
>>> On Wed, 30 Aug 2017 19:05:58 +0200
>>> David Hildenbrand  wrote:
>>>   
 Specifying more than 1 CPU (e.g. -smp 5) leads to SIGP errors (the
 guest tries to bring these CPUs up but fails), because we don't support
 multiple CPUs on s390x under TCG.

 Let's bail out if more than 1 are specified, so we don't raise people's
 hope.

 Signed-off-by: David Hildenbrand 
 ---
  hw/s390x/s390-virtio-ccw.c | 6 ++
  1 file changed, 6 insertions(+)

 diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
 index 7754e3eaf9..eff96808c4 100644
 --- a/hw/s390x/s390-virtio-ccw.c
 +++ b/hw/s390x/s390-virtio-ccw.c
 @@ -23,6 +23,7 @@
  #include "hw/s390x/css.h"
  #include "virtio-ccw.h"
  #include "qemu/config-file.h"
 +#include "qemu/error-report.h"
  #include "s390-pci-bus.h"
  #include "hw/s390x/storage-keys.h"
  #include "hw/s390x/storage-attributes.h"
 @@ -56,6 +57,11 @@ static void s390_init_cpus(MachineState *machine)
  if (machine->cpu_model == NULL) {
  machine->cpu_model = s390_default_cpu_model_name();
  }
 +if (tcg_enabled() && max_cpus > 1) {
 +error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
 + "supported by TCG (1) on s390x", max_cpus);  
>>>
>>> Make this a #define, so we can just flip the switch when smp support is
>>> ready?  
>>
>> As an alternative: yield a warning?
> 
> If we know that this can't work, it makes sense to stop immediately, no?

Hmm, like that than?

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 18ed0c57e3..dae848fa5f 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -23,6 +23,7 @@
 #include "hw/s390x/css.h"
 #include "virtio-ccw.h"
 #include "qemu/config-file.h"
+#include "qemu/error-report.h"
 #include "s390-pci-bus.h"
 #include "hw/s390x/storage-keys.h"
 #include "hw/s390x/storage-attributes.h"
@@ -56,6 +57,12 @@ static void s390_init_cpus(MachineState *machine)
 if (machine->cpu_model == NULL) {
 machine->cpu_model = s390_default_cpu_model_name();
 }
+if (tcg_enabled() && max_cpus > S390_TCG_MAX_CPUS) {
+error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
+ "supported by TCG (%d) on s390x", max_cpus,
+ S390_TCG_MAX_CPUS);
+exit(1);
+}

 ms->cpus = g_new0(S390CPU *, max_cpus);

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 7ed9103b33..4e1de2102d 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -210,6 +210,8 @@ static inline S390CPU
*s390_env_get_cpu(CPUS390XState *env)

 #define ENV_OFFSET offsetof(S390CPU, env)

+#define S390_TCG_MAX_CPUS 1
+
 #ifndef CONFIG_USER_ONLY
 extern const struct VMStateDescription vmstate_s390_cpu;
 #endif


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals

[Laszlo Ersek]

On 08/31/17 10:42, Markus Armbruster wrote:
> Marc-André Lureau  writes:
> 
>> Hi
>>
>> - Original Message -
>>> Marc-André Lureau  writes:
>>>
 Hi

 - Original Message -
> Marc-André Lureau  writes:
>
>> They are not considered constant expressions in C, producing an error
>> when compiling a const QLit.
>
> A const QLit?  Do you mean a non-const one?

 Really a const QLitObject:


 const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
  QLIT_QNULL,
  {}
  }));

 qmp-introspect.c:17:63: error: initializer element is not constant
   const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
 ^
 Removing the "compound literals" fixes this error.
>>>
>>> Does QLIT_QLIST(((const QLitObject[]) { ... } work?
>>
>> No. Even if I put "const" all over the place (in member, in compound type 
>> etc).
>>
>> Give it a try, see if you can make it const, I am out of luck.
> 
> The commit message's explanation is wrong.  This isn't about const at
> all, it's about "constant expressions", which are something else
> entirely.
> 
> For what it's worth, clang is cool with the compound literals.  On
> Fedora 26 with a minimized test case (appended):
> 
> $ clang -c -g -O -Wall compound-lit.c
> $ gcc -c -g -O -Wall compound-lit.c
> compound-lit.c:30:37: error: initializer element is not constant
>  .value.qdict = (QLitDictEntry[]){
>  ^
> compound-lit.c:30:37: note: (near initialization for ‘(anonymous).value’)
> 
> GCC bug or not?  A search of the GCC Bugzilla finds
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71713
> 
> Copying a few notorious language lawyers^W^W^Wtrusted advisers.
> 
> Even if this turns out to be a gcc bug, we'll have to work around it.
> But the work-around needs a comment then.
> 
> In any case, the commit message needs fixing.
> 
> 
> 
> enum {
> QTYPE_NONE, QTYPE_QSTRING, QTYPE_QDICT,
> };
> 
> typedef struct QLitDictEntry QLitDictEntry;
> typedef struct QLitObject QLitObject;
> 
> struct QLitObject {
> int type;
> union {
> const char *qstr;
> QLitDictEntry *qdict;
> } value;
> };
> 
> struct QLitDictEntry {
> const char *key;
> QLitObject value;
> };
> 
> QLitObject qlit1 = (QLitObject){
> .type = QTYPE_QDICT,
> .value.qdict = (QLitDictEntry[]){
>   { "foo", {} },
>   {}
> }};
> 
> QLitObject qlit2 = (QLitObject){
> .type = QTYPE_QDICT,
> .value.qdict = (QLitDictEntry[]){
>   { "foo", (QLitObject){} },
>   {}
> }};
> 

(1) When discussing standards conformance, please drop the {} construct;
it is a GNUism. Replacing it with { 0 } works in all contexts, and
conforms to the standard. (Not trying to be pedantic here, but it does
elicit extra warnings from my gcc command line

gcc -std=c99 -pedantic -Wall -Wextra -fsyntax-only


(2) Let's see what the standard says:

  6.5.2.5 Compound literals
  Constraints
  3 If the compound literal occurs outside the body of a function, the
initializer list shall consist of constant expressions.

In the initialization of "qlit1", one element of the initializer list
(namely for .value.qdict) is

[1] (QLitDictEntry[]) {
  { "foo", { 0 } },
  { 0 }
}

Is this a constant expression?

  6.6 Constant expressions
  7 More latitude is permitted for constant expressions in initializers.
Such a constant expression shall be, or evaluate to, one of the
following:
- an arithmetic constant expression,
- a null pointer constant,
- an address constant, or
- an address constant for an object type plus or minus an integer
  constant expression.

Now, is [1] an address constant?

  6.6 Constant expressions
  9 An address constant is a null pointer, a pointer to an lvalue
designating an object of static storage duration, or a pointer to a
function designator; it shall be created explicitly using the unary
& operator or an integer constant cast to pointer type, or
implicitly by the use of an expression of array or function type.
The array-subscript [] and member-access . and -> operators, the
address & and indirection * unary operators, and pointer casts may
be used in the creation of an address constant, but the value of an
object shall not be accessed by use of these operators.

"expression of array [...] type" applies; question is:
- is [1] an lvalue designating an object of static storage duration?

  6.5.2.5 Compound literals
  Semantics
  5 If the type name specifies an array of unknown size, the size is
determined by the initializer list as specified in 6.7.8, and the
type of the compound literal is that of the completed array type.
Otherwise (when the 

Re: [Qemu-devel] [PATCH 1/7] serial: add serial_chr_nonnull() to use the null backend when none provided

[Philippe Mathieu-Daudé]

On 08/31/2017 06:43 AM, Thomas Huth wrote:

On 31.08.2017 11:36, Marc-André Lureau wrote:

Hi

On Thu, Aug 31, 2017 at 7:20 AM Thomas Huth > wrote:

 On 31.08.2017 05:53, Philippe Mathieu-Daudé wrote:
 > Suggested-by: Peter Maydell >
 > Signed-off-by: Philippe Mathieu-Daudé >
 > ---
 >  include/hw/char/serial.h |  1 +
 >  hw/char/serial.c         | 13 +
 >  2 files changed, 14 insertions(+)
 >
 > diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
 > index c4daf11a14..96bccb39e1 100644
 > --- a/include/hw/char/serial.h
 > +++ b/include/hw/char/serial.h
 > @@ -93,6 +93,7 @@ SerialState *serial_mm_init(MemoryRegion
 *address_space,
 >                              hwaddr base, int it_shift,
 >                              qemu_irq irq, int baudbase,
 >                              Chardev *chr, enum device_endian end);
 > +Chardev *serial_chr_nonnull(Chardev *chr);

 Why do you need the prototype? Please make the function static if
 possible (otherwise please add some rationale in the patch description).

It's being used from other units in following patches


Ah, well, right. I was only on CC: in the first two patches, so I missed
the other ones at the first glance. So never mind my comment, the
prototype is fine here.


Is it better/easier to use the same list for the cover and all the patches?
I try to shorten the it to help overloaded reviewer to focus on the 
patches I think they can help. But this case show it's not that useful.

Re: [Qemu-devel] [PATCH 1/7] serial: add serial_chr_nonnull() to use the null backend when none provided

[Thomas Huth]

On 31.08.2017 17:20, Philippe Mathieu-Daudé wrote:
> On 08/31/2017 02:19 AM, Thomas Huth wrote:
>> On 31.08.2017 05:53, Philippe Mathieu-Daudé wrote:
> [...]>> +Chardev *serial_chr_nonnull(Chardev *chr)
>>> +{
>>> +    static int serial_id;
>>> +    char *label;
>>> +
>>> +    label = g_strdup_printf("discarding-serial%d", serial_id++);
>>> +    chr = qemu_chr_new(label, "null");
>>
>> That looks wrong - you're ignoring the input parameter and always open
>> the "null" device? Shouldn't there be a "if (chr) return chr;" in front
>> of this?
> 
> You right. I had this correct in my first patch when this code was
> embedded, I then failed at extracting as another function :/
> 
>>
>>> +    assert(chr);
>>> +    g_free(label);
>>> +
>>> +    return chr;
>>> +}
>>
>>   Thomas
>>
>> PS: I think you should also merge the two patches together, they are
>> small enough.
> 
> Ok.

Well, I wrote that comment about merging the two patches together when I
was thinking that your series consists of only two patches (since I've
only been CC:-ed on the first two patches). So please simply ignore that
suggestion :-)

 Thomas

Re: [Qemu-devel] [PATCH v2 4/4] block: convert qcrypto_block_encrypt|decrypt to take bytes offset

[Daniel P. Berrange]

On Thu, Aug 31, 2017 at 10:17:43AM -0500, Eric Blake wrote:
> On 08/31/2017 06:05 AM, Daniel P. Berrange wrote:
> > Instead of sector offset, take the bytes offset when encrypting
> > or decrypting data.
> > 
> > Signed-off-by: Daniel P. Berrange 
> > ---
> >  block/crypto.c |  8 
> >  block/qcow.c   |  7 +--
> >  block/qcow2-cluster.c  |  8 +++-
> >  block/qcow2.c  |  4 ++--
> >  crypto/block-luks.c| 12 
> >  crypto/block-qcow.c| 12 
> >  crypto/block.c | 14 --
> >  crypto/blockpriv.h |  4 ++--
> >  include/crypto/block.h | 14 --
> >  9 files changed, 48 insertions(+), 35 deletions(-)
> > 
> > diff --git a/block/crypto.c b/block/crypto.c
> > index 40daa77188..6b8d88efbc 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -426,8 +426,8 @@ block_crypto_co_preadv(BlockDriverState *bs, uint64_t 
> > offset, uint64_t bytes,
> >  goto cleanup;
> >  }
> >  
> > -if (qcrypto_block_decrypt(crypto->block, sector_num, cipher_data,
> > -  cur_bytes, NULL) < 0) {
> > +if (qcrypto_block_decrypt(crypto->block, offset + bytes_done,
> > +  cipher_data, cur_bytes, NULL) < 0) {
> 
> How close are we to getting rid of even needing 'sector_num' as a variable?

I thought there was more usage, but I just realized we can in fact
remove it in this patch.

> 
> > +++ b/block/qcow.c
> > @@ -456,7 +456,9 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
> >  if (i < n_start || i >= n_end) {
> >  Error *err = NULL;
> >  memset(s->cluster_data, 0x00, 512);
> > -if (qcrypto_block_encrypt(s->crypto, 
> > start_sect + i,
> > +if (qcrypto_block_encrypt(s->crypto,
> > +  start_sect + i *
> > +  BDRV_SECTOR_SIZE,
> 
> Umm, not the same.  You want (start_sect + i) * BDRV_SECTOR_SIZE.

Heh, oppps.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 1/7] serial: add serial_chr_nonnull() to use the null backend when none provided

[Philippe Mathieu-Daudé]

On 08/31/2017 02:19 AM, Thomas Huth wrote:

On 31.08.2017 05:53, Philippe Mathieu-Daudé wrote:

[...]>> +Chardev *serial_chr_nonnull(Chardev *chr)

+{
+static int serial_id;
+char *label;
+
+label = g_strdup_printf("discarding-serial%d", serial_id++);
+chr = qemu_chr_new(label, "null");


That looks wrong - you're ignoring the input parameter and always open
the "null" device? Shouldn't there be a "if (chr) return chr;" in front
of this?


You right. I had this correct in my first patch when this code was 
embedded, I then failed at extracting as another function :/





+assert(chr);
+g_free(label);
+
+return chr;
+}


  Thomas

PS: I think you should also merge the two patches together, they are
small enough.


Ok.

Re: [Qemu-devel] [PATCH v8 0/5] hypertrace: Lightweight guest-to-QEMU trace channel

[Lluís Vilanova]

Stefan Hajnoczi writes:

> On Fri, Aug 25, 2017 at 08:34:54PM -0400, Emilio G. Cota wrote:
>> On Sun, Jul 30, 2017 at 17:08:18 +0300, Lluís Vilanova wrote:
>> > The hypertrace channel allows guest code to emit events in QEMU (the host) 
>> > using
>> > its tracing infrastructure (see "docs/trace.txt"). This works in both 
>> > 'system'
>> > and 'user' modes, is architecture-agnostic and introduces minimal noise on 
>> > the
>> > guest.
>> > 
>> > See first commit for a full description, use-cases and an example.
>> > 
>> > Signed-off-by: Lluís Vilanova 
>> 
>> This would be indeed very useful once TCG instrumentation is in place.
>> 
>> However, I'm not very excited about this being PCI-only and Linux-only for
>> system mode
>> I wonder how we could make this work on all hosts -- did you consider using
>> "magic" instructions? We'd need a different magic instruction for each
>> guest ISA, but the library would hide that anyway (and the library code
>> would be the same for user and system modes).

> Magic instructions can potentially be implemented more efficiently in
> TCG too.  They wouldn't work under KVM/HAX/hvf accelerators though.

Yes, we discussed this on the first version of this series long ago. The special
device makes it work on both TCG and KVM (if it were possible to implement a
TCG-to/from-KVM mode switch that'd be awesome).

Also, both approaches have negligible performance impact on the *guest* time.


Cheers,
  Lluis

Re: [Qemu-devel] [PATCH v2 4/4] block: convert qcrypto_block_encrypt|decrypt to take bytes offset

[Eric Blake]

On 08/31/2017 06:05 AM, Daniel P. Berrange wrote:
> Instead of sector offset, take the bytes offset when encrypting
> or decrypting data.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  block/crypto.c |  8 
>  block/qcow.c   |  7 +--
>  block/qcow2-cluster.c  |  8 +++-
>  block/qcow2.c  |  4 ++--
>  crypto/block-luks.c| 12 
>  crypto/block-qcow.c| 12 
>  crypto/block.c | 14 --
>  crypto/blockpriv.h |  4 ++--
>  include/crypto/block.h | 14 --
>  9 files changed, 48 insertions(+), 35 deletions(-)
> 
> diff --git a/block/crypto.c b/block/crypto.c
> index 40daa77188..6b8d88efbc 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -426,8 +426,8 @@ block_crypto_co_preadv(BlockDriverState *bs, uint64_t 
> offset, uint64_t bytes,
>  goto cleanup;
>  }
>  
> -if (qcrypto_block_decrypt(crypto->block, sector_num, cipher_data,
> -  cur_bytes, NULL) < 0) {
> +if (qcrypto_block_decrypt(crypto->block, offset + bytes_done,
> +  cipher_data, cur_bytes, NULL) < 0) {

How close are we to getting rid of even needing 'sector_num' as a variable?

> +++ b/block/qcow.c
> @@ -456,7 +456,9 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
>  if (i < n_start || i >= n_end) {
>  Error *err = NULL;
>  memset(s->cluster_data, 0x00, 512);
> -if (qcrypto_block_encrypt(s->crypto, start_sect 
> + i,
> +if (qcrypto_block_encrypt(s->crypto,
> +  start_sect + i *
> +  BDRV_SECTOR_SIZE,

Umm, not the same.  You want (start_sect + i) * BDRV_SECTOR_SIZE.

> +++ b/block/qcow2-cluster.c
> @@ -396,15 +396,13 @@ static bool coroutine_fn 
> do_perform_cow_encrypt(BlockDriverState *bs,
>  {
>  if (bytes && bs->encrypted) {
>  BDRVQcow2State *s = bs->opaque;
> -int64_t sector = (s->crypt_physical_offset ?
> +int64_t offset = (s->crypt_physical_offset ?
>(cluster_offset + offset_in_cluster) :
> -  (src_cluster_offset + offset_in_cluster))
> - >> BDRV_SECTOR_BITS;
> +  (src_cluster_offset + offset_in_cluster));
>  assert((offset_in_cluster & ~BDRV_SECTOR_MASK) == 0);
>  assert((bytes & ~BDRV_SECTOR_MASK) == 0);

Pre-existing, but we could use osdep.h macros here, as in QEMU_IS_ALIGNED().

> +++ b/crypto/block-luks.c
> @@ -1403,29 +1403,33 @@ static void qcrypto_block_luks_cleanup(QCryptoBlock 
> *block)
>  
>  static int
>  qcrypto_block_luks_decrypt(QCryptoBlock *block,
> -   uint64_t startsector,
> +   uint64_t offset,
> uint8_t *buf,
> size_t len,
> Error **errp)
>  {
> +assert(!(offset % QCRYPTO_BLOCK_LUKS_SECTOR_SIZE));
> +assert(!(len % QCRYPTO_BLOCK_LUKS_SECTOR_SIZE));

Again, QEMU_IS_ALIGNED() might be easier to read - but this time, it's
in code you're adding.

Looking forward to v3.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/7] serial: add serial_chr_nonnull() to use the null backend when none provided

[Philippe Mathieu-Daudé]

On 08/31/2017 07:28 AM, Peter Maydell wrote:

On 31 August 2017 at 04:53, Philippe Mathieu-Daudé  wrote:

Suggested-by: Peter Maydell 
Signed-off-by: Philippe Mathieu-Daudé 
---
  include/hw/char/serial.h |  1 +
  hw/char/serial.c | 13 +
  2 files changed, 14 insertions(+)

diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
index c4daf11a14..96bccb39e1 100644
--- a/include/hw/char/serial.h
+++ b/include/hw/char/serial.h
@@ -93,6 +93,7 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
  hwaddr base, int it_shift,
  qemu_irq irq, int baudbase,
  Chardev *chr, enum device_endian end);
+Chardev *serial_chr_nonnull(Chardev *chr);


For new globally-visible function declarations, can we
have a doc-comment form comment that documents the
function, please?


I was afraid someone asked that, but since this file has no other 
comment I tried :p


Good to know for future contributions, someone fluent with English 
should add this to CODING_STYLE.

Re: [Qemu-devel] [PATCH v2 3/4] block: convert crypto driver to bdrv_co_preadv|pwritev

[Eric Blake]

On 08/31/2017 06:05 AM, Daniel P. Berrange wrote:
> Make the crypto driver implement the bdrv_co_preadv|pwritev
> callbacks,  and also use bdrv_co_preadv|pwritev for I/O
> with the protocol driver beneath.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  block/crypto.c | 103 
> +
>  1 file changed, 53 insertions(+), 50 deletions(-)
> 

>  static coroutine_fn int
> -block_crypto_co_readv(BlockDriverState *bs, int64_t sector_num,
> -  int remaining_sectors, QEMUIOVector *qiov)
> +block_crypto_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
> +   QEMUIOVector *qiov, int flags)

>  {
>  BlockCrypto *crypto = bs->opaque;
> -int cur_nr_sectors; /* number of sectors in current iteration */
> +uint64_t cur_bytes; /* number of bytes in current iteration */
>  uint64_t bytes_done = 0;
>  uint8_t *cipher_data = NULL;
>  QEMUIOVector hd_qiov;
>  int ret = 0;
> -size_t payload_offset =
> -qcrypto_block_get_payload_offset(crypto->block) / BDRV_SECTOR_SIZE;
> +size_t payload_offset = qcrypto_block_get_payload_offset(crypto->block);

Pre-existing: is size_t the right type, or can we overflow a 64-bit
offset on a 32-bit host?

> +uint64_t sector_num = offset / BDRV_SECTOR_SIZE;
> +
> +assert((offset % BDRV_SECTOR_SIZE) == 0);
> +assert((bytes % BDRV_SECTOR_SIZE) == 0);

The osdep.h macros might be nicer than open-coding; furthermore, if
desired, you could shorten to:

assert(QEMU_IS_ALIGNED(offset | bytes, BDRV_SECTOR_SIZE));


>  
>  static coroutine_fn int
> -block_crypto_co_writev(BlockDriverState *bs, int64_t sector_num,
> -   int remaining_sectors, QEMUIOVector *qiov)
> +block_crypto_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t 
> bytes,
> +QEMUIOVector *qiov, int flags)
>  {

Hmm - you don't set supported_write_flags.  But presumably, if the
underlying BDS supports BDRV_REQUEST_FUA, then crypto can likewise
support that flag by passing it through to the underlying device after
encryption.

>  BlockCrypto *crypto = bs->opaque;
> -int cur_nr_sectors; /* number of sectors in current iteration */
> +uint64_t cur_bytes; /* number of bytes in current iteration */
>  uint64_t bytes_done = 0;
>  uint8_t *cipher_data = NULL;
>  QEMUIOVector hd_qiov;
>  int ret = 0;
> -size_t payload_offset =
> -qcrypto_block_get_payload_offset(crypto->block) / BDRV_SECTOR_SIZE;
> +size_t payload_offset = qcrypto_block_get_payload_offset(crypto->block);
> +uint64_t sector_num = offset / BDRV_SECTOR_SIZE;
> +
> +assert((offset % BDRV_SECTOR_SIZE) == 0);
> +assert((bytes % BDRV_SECTOR_SIZE) == 0);

Same comment as for read.

> @@ -611,8 +613,9 @@ BlockDriver bdrv_crypto_luks = {
>  .bdrv_truncate  = block_crypto_truncate,
>  .create_opts= _crypto_create_opts_luks,
>  
> -.bdrv_co_readv  = block_crypto_co_readv,
> -.bdrv_co_writev = block_crypto_co_writev,
> +.bdrv_refresh_limits = block_crypto_refresh_limits,
> +.bdrv_co_preadv  = block_crypto_co_preadv,
> +.bdrv_co_pwritev = block_crypto_co_pwritev,
>  .bdrv_getlength = block_crypto_getlength,
>  .bdrv_get_info  = block_crypto_get_info_luks,
>  .bdrv_get_specific_info = block_crypto_get_specific_info_luks,

Looks weird when = isn't consistently aligned, but we use more than one
space.  My preference is to just use one space everywhere, as adding a
longer name to the list doesn't require a mass re-format of all other
entries; but I'm not opposed when people like the aligned = for
legibility.  So up to you if you do anything in response to my nit.

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v1 08/11] s390x: allow only 1 CPU with TCG

[Cornelia Huck]

On Thu, 31 Aug 2017 17:03:21 +0200
David Hildenbrand  wrote:

> On 31.08.2017 16:41, Cornelia Huck wrote:
> > On Wed, 30 Aug 2017 19:05:58 +0200
> > David Hildenbrand  wrote:
> >   
> >> Specifying more than 1 CPU (e.g. -smp 5) leads to SIGP errors (the
> >> guest tries to bring these CPUs up but fails), because we don't support
> >> multiple CPUs on s390x under TCG.
> >>
> >> Let's bail out if more than 1 are specified, so we don't raise people's
> >> hope.
> >>
> >> Signed-off-by: David Hildenbrand 
> >> ---
> >>  hw/s390x/s390-virtio-ccw.c | 6 ++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> >> index 7754e3eaf9..eff96808c4 100644
> >> --- a/hw/s390x/s390-virtio-ccw.c
> >> +++ b/hw/s390x/s390-virtio-ccw.c
> >> @@ -23,6 +23,7 @@
> >>  #include "hw/s390x/css.h"
> >>  #include "virtio-ccw.h"
> >>  #include "qemu/config-file.h"
> >> +#include "qemu/error-report.h"
> >>  #include "s390-pci-bus.h"
> >>  #include "hw/s390x/storage-keys.h"
> >>  #include "hw/s390x/storage-attributes.h"
> >> @@ -56,6 +57,11 @@ static void s390_init_cpus(MachineState *machine)
> >>  if (machine->cpu_model == NULL) {
> >>  machine->cpu_model = s390_default_cpu_model_name();
> >>  }
> >> +if (tcg_enabled() && max_cpus > 1) {
> >> +error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
> >> + "supported by TCG (1) on s390x", max_cpus);  
> > 
> > Make this a #define, so we can just flip the switch when smp support is
> > ready?  
> 
> As an alternative: yield a warning?

If we know that this can't work, it makes sense to stop immediately, no?

> 
> >   
> >> +exit(1);
> >> +}
> >>  
> >>  ms->cpus = g_new0(S390CPU *, max_cpus);
> >>
> >   
> 
> 

Re: [Qemu-devel] [PATCH v1 08/11] s390x: allow only 1 CPU with TCG

[David Hildenbrand]

On 31.08.2017 16:41, Cornelia Huck wrote:
> On Wed, 30 Aug 2017 19:05:58 +0200
> David Hildenbrand  wrote:
> 
>> Specifying more than 1 CPU (e.g. -smp 5) leads to SIGP errors (the
>> guest tries to bring these CPUs up but fails), because we don't support
>> multiple CPUs on s390x under TCG.
>>
>> Let's bail out if more than 1 are specified, so we don't raise people's
>> hope.
>>
>> Signed-off-by: David Hildenbrand 
>> ---
>>  hw/s390x/s390-virtio-ccw.c | 6 ++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 7754e3eaf9..eff96808c4 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -23,6 +23,7 @@
>>  #include "hw/s390x/css.h"
>>  #include "virtio-ccw.h"
>>  #include "qemu/config-file.h"
>> +#include "qemu/error-report.h"
>>  #include "s390-pci-bus.h"
>>  #include "hw/s390x/storage-keys.h"
>>  #include "hw/s390x/storage-attributes.h"
>> @@ -56,6 +57,11 @@ static void s390_init_cpus(MachineState *machine)
>>  if (machine->cpu_model == NULL) {
>>  machine->cpu_model = s390_default_cpu_model_name();
>>  }
>> +if (tcg_enabled() && max_cpus > 1) {
>> +error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
>> + "supported by TCG (1) on s390x", max_cpus);
> 
> Make this a #define, so we can just flip the switch when smp support is
> ready?

As an alternative: yield a warning?

> 
>> +exit(1);
>> +}
>>  
>>  ms->cpus = g_new0(S390CPU *, max_cpus);
>>  
> 


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v2 2/4] block: use BDRV_SECTOR_SIZE in crypto driver

[Eric Blake]

On 08/31/2017 06:05 AM, Daniel P. Berrange wrote:
> Signed-off-by: Daniel P. Berrange 
> ---
>  block/crypto.c | 32 
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v5 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux)

[Kamil Rytarowski]

On 31.08.2017 16:27, Peter Maydell wrote:
> On 31 August 2017 at 15:04, Kamil Rytarowski  wrote:
>> Could we please add SmartOS image. It's an Illumos distribution,
>> previously known as OpenSolaris.
>>
>> qemu is actively maintained for SmartOS in pkgsrc.
>>
>> The latest build fixes were introduced less than a week ago.
>> https://github.com/NetBSD/pkgsrc/commit/f2608af5433f1491a92567bf570e9349dffbda7a
> 
> Maintaining out of tree patches without posting them upstream
> is rather counterproductive, because if the host OS doesn't
> build or have anybody submitting patches to fix it upstream
> we will conclude that it's unused and delete the support entirely.
> That E1000_SEC define has been around for nearly 10 years
> without anybody saying it was a problem, for instance.
> (Indeed Solaris is probably next after AIX on the "nobody
> came along to tell us they cared enough to help us maintain
> it" list for removal as a result.)
> 

Fully agreed. I keep slowly unloading pkgsrc from the downstream patches.

>> I'm going to submit the SmartOS patches to fix the build
>> regardless of the decision.
> 
> Thank you. Do you also have some instructions on how to
> get it running in a VM or something so we can add it to
> our build tests? (Login on a build machine, or a SmartOS
> host in the gcc compile farm, would work too).
> 

I use a Joyent machine for pkgsrc developers. I'm right now test
building 2.10.0 (including the dependencies).

I don't have any first-time starter.. This wiki might be useful.
https://wiki.smartos.org/display/DOC/Download+SmartOS

SmartOS is a regular UNIX-like OS (or even certified UNIX-based).

> thanks
> -- PMM
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL 00/11] NBD patches for 2.11

[Peter Maydell]

On 30 August 2017 at 19:07, Eric Blake  wrote:
> The following changes since commit 1ab5eb4efb91a3d4569b0df6e824cc08ab4bd8ec:
>
>   Update version for v2.10.0 release (2017-08-30 17:02:54 +0100)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/ericb.git tags/pull-nbd-2017-08-30
>
> for you to fetch changes up to f35dff7e13b84d3fffe1103c2c69afd81df5e4f5:
>
>   block/nbd-client: refactor request send/receive (2017-08-30 13:00:38 -0500)
>
> 
> nbd patches for 2017-08-30
>
> - Kashyap Chamarthy: qemu-iotests: Extend non-shared storage migration test 
> (194)
> - Stefan Hajnaczi: 0/3 nbd-client: enter read_reply_co during init to avoid 
> crash
> - Vladimir Sementsov-Ogievskiy: portions of 0/17 nbd client refactoring and 
> fixing
>
> 

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL for-2.10 00/15] Block patches

[Peter Maydell]

On 31 August 2017 at 09:21, Stefan Hajnoczi  wrote:
> The following changes since commit 248b23735645f7cbb503d9be6f5bf825f2a603ab:
>
>   Update version for v2.10.0-rc4 release (2017-08-24 17:34:26 +0100)
>
> are available in the git repository at:
>
>   git://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 3e4c705212abfe8c9882a00beb2d1466a8a53cec:
>
>   qcow2: allocate cluster_cache/cluster_data on demand (2017-08-30 18:02:10 
> +0100)
>
> 
>
> 

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v1 06/11] target/s390x: cleanup cpu number/address handling

[Cornelia Huck]

On Thu, 31 Aug 2017 16:41:47 +0200
David Hildenbrand  wrote:

> On 31.08.2017 16:35, Cornelia Huck wrote:
> > On Wed, 30 Aug 2017 19:05:56 +0200
> > David Hildenbrand  wrote:
> >   
> >> Some time ago we discussed that using "id" as property name is not the
> >> right thing to do, as it is a reserved property for other devices.
> >>
> >> Switch to the term "addr" instead, which matches the definition in the
> >> PoP called "CPU address". There is no such thing as cpu number, so
> >> rename env.cpu_num to env.cpu_addr.
> >>
> >> We can get rid of cpu->id now. Keep cpu->index and env->cpu_addr in sync.
> >> cpu->index was already implicitly used by e.g. cpu_exists(), so keeping
> >> both in sync seems to be the right thing to do.
> >>
> >> cpu->index will now no longer automatically get set via
> >> cpu_exec_realizefn(). For now, we were lucky that both implicitly stayed
> >> in sync.
> >>
> >> Our new cpu property "addr" can be a static property. Range checks can
> >> be avoided by using the correct type and the "setting after realized"
> >> check is done implicitly.
> >>
> >> AFAIK, s390x only supports cpu_add and not device_add for cpus. So we
> >> should be able to safely rename that property (no the "id" property
> >> could properly be used for device_add, which needs an artificial id for
> >> identification purposes).  
> > 
> > I cannot parse the sentence in the brackets...  
> 
> Me too :)
> 
> ...So we should be able to safely rename that property. device_add will
> later need the reserved "id" property. Hotplugging a CPU would then look
> like this: "device_add host-s390-cpu id=cpu2 addr=2".

Yup, that's understandable :)

Re: [Qemu-devel] [PATCH v5 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux)

[Fam Zheng]

On Thu, 08/31 16:04, Kamil Rytarowski wrote:
> Could we please add SmartOS image. It's an Illumos distribution,
> previously known as OpenSolaris.
> 
> qemu is actively maintained for SmartOS in pkgsrc.
> 
> The latest build fixes were introduced less than a week ago.
> https://github.com/NetBSD/pkgsrc/commit/f2608af5433f1491a92567bf570e9349dffbda7a
> 
> I'm going to submit the SmartOS patches to fix the build regardless of
> the decision.

When QEMU builds on it, and when someone composes an instruction page like 

> > https://wiki.qemu.org/Hosts/BSD

then probably yes.

Fam

Re: [Qemu-devel] [PATCH v1 03/11] s390x: store cpu states inside machine state

[Thomas Huth]

On 31.08.2017 16:36, David Hildenbrand wrote:
> On 31.08.2017 16:31, Thomas Huth wrote:
>> On 31.08.2017 16:23, David Hildenbrand wrote:
>>>
> +struct S390CPU;

 You define a "struct S390CPU" here ...

>  typedef struct S390CcwMachineState {
>  /*< private >*/
>  MachineState parent_obj;
>  
>  /*< public >*/
> +S390CPU **cpus;

 ... but use the typedef'ed S390CPU here ... looks somewhat suspicious, I
 wonder whether the typedef is really in the right place?
>>>
>>> General question: how much do we care about headers that are not consistent?
>>>
>>> E.g. shall I forward declare or simply ignore if compilers don't bite me?
>>
>> My remark was not so much about your patch, but about the original
>> definition instead: "struct S390CPU" is declared in target/s390x/cpu.h,
>> but "typedef struct S390CPU S390CPU" is in target/s390x/cpu-qom.h. I
>> think they should rather be declared in the same header file instead. Or
> 
> I agree, will have a look.
> 
>> your "struct S390CPU;" forward declaration should go into cpu-qom.h
>> instead, right in front of the typedef.
>>
> 
> Let me rephrase my question:
> 
> include/hw/s390x/s390-virtio-ccw.h does not include cpu.h/cpu-qom.h
> 
> If compilers don't complain, do we have to forward declare at all? (I
> think it is cleaner, but I would like to know what is suggested)

Well, doing a forward declaration with "struct S390CPU;" and then using
the typedef'd "S390CPU" without "struct" does not make much sense -
these are two "different" types. If you can use "S390CPU" here without
the "struct S390CPU;" declaration, that means the cpu-qom.h header got
included indirectly already - so I'd suggest to simply remove the
"struct S390CPU;" forward declaration from your patch.

 Thomas

Re: [Qemu-devel] [PATCH v1 00/11] next round of s390x cleanups

[Cornelia Huck]

On Wed, 30 Aug 2017 19:05:50 +0200
David Hildenbrand  wrote:

> No new functionality, only cleanups, some of the discussed during the last
> round of cleanups.
> 
> Not sure if the first two patches should be sent separatly? Anyhow, they
> are most probably not worth the trouble :)

If you want to clean up the other unneeded includes of cpu-all.h as
well, it might make sense to send them separately. Otherwise, I'm happy
to route them through s390-next if nobody complains.

> 
> The biggest part of this series is getting rid of s390-virtio.c and
> cleaning up our cpu number/id handling.
> 
> Based on: git://github.com/cohuck/qemu s390-next

Looking forward to v2 :)

> 
> 
> David Hildenbrand (11):
>   exec,dump: don't include exec/exec-all.h explicitly
>   cpu: drop old comments describing members
>   s390x: store cpu states inside machine state
>   s390x: get rid of s390-virtio.c
>   s390x: rename s390-virtio.h to s390-virtio-hcall.h
>   target/s390x: cleanup cpu number/address handling
>   target/s390x: rename next_cpu_id to next_cpu_addr
>   s390x: allow only 1 CPU with TCG
>   target/s390x: tcg_s390_program_interrupt() will never return
>   target/s390x: use trigger_pgm_exception() in
> s390_cpu_handle_mmu_fault()
>   target/s390x: use program_interrupt() in per_check_exception()
> 
>  dump.c |   1 -
>  exec.c |   1 -
>  hw/s390x/Makefile.objs |   1 -
>  hw/s390x/s390-virtio-ccw.c | 169 ++-
>  hw/s390x/s390-virtio-hcall.c   |   2 +-
>  hw/s390x/s390-virtio-hcall.h   |  20 
>  hw/s390x/s390-virtio.c | 201 
> -
>  hw/s390x/s390-virtio.h |  35 ---
>  include/hw/s390x/s390-virtio-ccw.h |   3 +
>  include/qom/cpu.h  |   6 +-
>  target/s390x/cpu-qom.h |   2 +-
>  target/s390x/cpu.c |  74 --
>  target/s390x/cpu.h |   5 +-
>  target/s390x/cpu_models.c  |   2 +-
>  target/s390x/excp_helper.c |   5 +-
>  target/s390x/helper.c  |   8 +-
>  target/s390x/interrupt.c   |   3 +-
>  target/s390x/misc_helper.c |  13 +--
>  target/s390x/translate.c   |   5 +-
>  19 files changed, 230 insertions(+), 326 deletions(-)
>  create mode 100644 hw/s390x/s390-virtio-hcall.h
>  delete mode 100644 hw/s390x/s390-virtio.c
>  delete mode 100644 hw/s390x/s390-virtio.h
> 

Re: [Qemu-devel] [PATCH v1 06/11] target/s390x: cleanup cpu number/address handling

[David Hildenbrand]

On 31.08.2017 16:35, Cornelia Huck wrote:
> On Wed, 30 Aug 2017 19:05:56 +0200
> David Hildenbrand  wrote:
> 
>> Some time ago we discussed that using "id" as property name is not the
>> right thing to do, as it is a reserved property for other devices.
>>
>> Switch to the term "addr" instead, which matches the definition in the
>> PoP called "CPU address". There is no such thing as cpu number, so
>> rename env.cpu_num to env.cpu_addr.
>>
>> We can get rid of cpu->id now. Keep cpu->index and env->cpu_addr in sync.
>> cpu->index was already implicitly used by e.g. cpu_exists(), so keeping
>> both in sync seems to be the right thing to do.
>>
>> cpu->index will now no longer automatically get set via
>> cpu_exec_realizefn(). For now, we were lucky that both implicitly stayed
>> in sync.
>>
>> Our new cpu property "addr" can be a static property. Range checks can
>> be avoided by using the correct type and the "setting after realized"
>> check is done implicitly.
>>
>> AFAIK, s390x only supports cpu_add and not device_add for cpus. So we
>> should be able to safely rename that property (no the "id" property
>> could properly be used for device_add, which needs an artificial id for
>> identification purposes).
> 
> I cannot parse the sentence in the brackets...

Me too :)

...So we should be able to safely rename that property. device_add will
later need the reserved "id" property. Hotplugging a CPU would then look
like this: "device_add host-s390-cpu id=cpu2 addr=2".

> 
>>
>> Signed-off-by: David Hildenbrand 
>> ---
>>  hw/s390x/s390-virtio-ccw.c |  2 +-
>>  target/s390x/cpu.c | 69 
>> --
>>  target/s390x/cpu.h |  5 ++--
>>  target/s390x/cpu_models.c  |  2 +-
>>  target/s390x/excp_helper.c |  2 +-
>>  target/s390x/helper.c  |  4 +--
>>  target/s390x/misc_helper.c |  4 +--
>>  target/s390x/translate.c   |  5 +---
>>  8 files changed, 28 insertions(+), 65 deletions(-)
> 
> ...the patch seems fine, though :)
> 


-- 

Thanks,

David